MattDiephouse

Reviewer of PRs.

Maintainer for Carthage, ReactiveCocoa, ReactiveSwift, PersistDB.

Ex-GitHub, ex-Apple.

I'm passionate about writing great software.

Most Recent

  1. » Types as Proof
  2. » Value-Oriented Programming
  3. » Eliminating Impossible States with Never
  4. » Better, Faster, Cheaper
  5. » When Not to Use an Enum

All Articles

Entry 5: Enum Element Diagnostics

16 Jul 2018

As I look for Swift bugs to work on, I’ve been testing bugs to see if they’re still reproducible. Many of them aren’t—particularly old type-checker issues—so they can be closed.

Take SR-4270 as an example.

let x: Bool = true
enum A {
    case a
    case b
}
func f(_ a: A) {}
let a: A = .a
f(x ? .a(a) : .b) // .a(a) is wrong. A.a takes no arguments.

This used to produce a terrible diagnostic:

error: result values in '? :' expression have mismatching types '' and ''
f(x ? .a(a) : .b)
~~~~~ ^ ~~

But in Swift 4.2, the diagnostic is much better:

error: member 'a' takes no arguments
f(x ? .a(a) : .b)
       ^~~~

I closed the bug as cannot reproduce, but Jordan Rose suggested that it could be even better if it explicitly mentioned that it was part of an enum. Something like enum element 'a' does not have an associated value.

I’m going to do that. But before I got started on it, I decided to look at enum element. When I’m looking at Swift errors, I usually infer quite a bit based on context. It can be hard to critically examine the error messages while I’m using them. But enum element seemed really strange now that I was thinking critically.

Looking through the other diagnostics, there were a few that mentioned enum element. But it seemed much clearer, at least to me, to say enum case instead.

By looking at the code that emitted these diagnostics, I found that the compiler has both EnumCaseDecls and EnumElementDecls in its AST. That’s because a case can actually name multiple elements:

enum Planet {
    case mercury, venus, earth, mars, jupiter, saturn, uranus, neptune
}

I don’t think I’d ever write an enum like that, so I’d forgotten about it. But it makes sense that the compiler would need different names for these internally. But using internal names from the compiler usually doesn’t make for clear diagnostics.

I searched for swift enum element, to see if anyone else used this term, and the only results were people asking what these error messages meant. That’s a good sign that they could be improved. But I also searched the Swift documentation about enums. The only use of element there is describing the contents of an array. Additionally, the Planet sample I copied above had this description:

Multiple cases can appear on a single line, separated by commas

That seemed like pretty good evidence that enum case would be a better diagnostic:

  1. There’s no evidence that enum element is a widely recognized/used term
  2. The documentation never uses that term
  3. The documentation explicitly calls these cases

So I decided to go ahead and open a PR to do that.

I used ag to search for occurrences of enum element. I found one diagnostic that used it directly, so I fixed that one.

But I also looked through the tests directory and found some other diagnostics that did. After fixing up those test expectations, I went to look for where they were emitted. They hadn’t turned up in my search. Here’s one example:

@objc
enum BadEnum2: Int {
  @objc(X:)   // expected-error{{'@objc' enum element must have a simple name}}{{10-11=}}
  case X
}

I searched for that diagnostic and found that it didn’t use enum element directly:

ERROR(objc_name_req_nullary,none,
      "'@objc' %0 must have a simple name", (DescriptiveDeclKind))

DescriptiveDeclKind here is the type of parameter that’s being passed to the format string. From previous fixes I’ve made, I knew that the EnumElementDecl must be getting passed to this—and something was rendering that as enum element.

So I did the simple thing: I searched the code for ”enum element”. Luckily, that turned up something that looked like what I wanted:

StringRef Decl::getDescriptiveKindName(DescriptiveDeclKind K) {
#define ENTRY(Kind, String) case DescriptiveDeclKind::Kind: return String
  switch (K) {
  … // other cases omitted for brevity
  ENTRY(EnumElement, "enum element");
  }
}

I had a feeling that changing this to ”enum case” would do what I wanted. I changed it, ran utils/build-script --debug --test, and waited.

That worked! I had to fix up a couple of tests, but I was ready to open a PR. You can view it here.

I think improving diagnostics is great way to contribute to the Swift compiler. It’s comparatively quite easy—in some cases it’s literally just editing strings! I also find it compelling because you can (a) eliminate confusion and (b) increase programmer productivity. The latter hopefully means that we can write better software.

If you have to google or ask about an error message, then it can be improved. So anytime you do, consider opening a PR to fix it!