Patterns: unhelpful parse errors when case clause const expression is not valid

[keertip]

Seeing Parse errors for this code:

void f(Object? x) {
  switch (x) {
    case 5 * 5:
      break;
  }
}

Other const expressions under switch statement in spec.

[keertip]

@stereotype441, please re-assign as needed.

[bwilkerson]

@scheglov fyi

[stereotype441]

This is expected behaviour with patterns enabled. See the spec section "Breaking existing switches", specifically this paragraph:

• Other constant expressions. Constant patterns allow simple literals and references to named constants to be used directly as patterns, which covers the majority of all existing switch cases. Also a constant constructor explicitly prefixed with const is a valid constant expression pattern. But some more complex expressions are valid constant expressions but not valid constant patterns. In the switch cases I analyzed, the exceptions are:

  case A + A:                                         // Infix "+".
  case A + 'b':                                       // Infix "+".
  case -ERR_LDS_ICAO_SIGNED_DATA_SIGNER_INFOS_EMPTY:  // Unary "-".
  case -sigkill:                                      // Unary "-".
  case List<RPChoice>:                                // Generic type literal.
  case 720 * 1280:                                    // Infix "*".
  case 1080 * 1920:                                   // Infix "*".
  case 1440 * 2560:                                   // Infix "*".
  case 2160 * 3840:                                   // Infix "*".
  

  These nine cases represent 0.009% of the cases found.

As the spec mentions, the workaround is to wrap the constant in const (...), i.e.:

void f(Object? x) {
  switch (x) {
    case const (5 * 5):
      break;
  }
}

[keertip]

The parse errors are not helpful and do not give the user any information of how to fix the issue. The analyzer can show more useful diagnostics here if the code can be parsed, giving the user better experience in this case.

[stereotype441]

@keertip ok, thanks, I understand now.

Unfortunately, improving the parse errors will probably be difficult bug to fix because of architectural limitations of the parser. As currently designed, the parsing logic for patterns and for expressions is entirely separate, and the parser chooses which logic to use after a case keyword based on whether pattern support is enabled. This means that in an example like case 5 * 5:, when the parser reaches the *, it's not able to see that the user tried to use an expression where a pattern was required; all it knows is that it got an unexpected token.

I'm open to ideas for how to improve this, just trying to set expectations 😃. CC @jensjoha or @johnniwinther in case either of them have ideas for how to improve this situation.

[bwilkerson]

Would it be possible, at the point where it sees a * (or any token it isn't expecting), for it to go back to the token after case, try skipping an expression, and see whether the next token is a :? If so, then it would make sense for it to report that the expression isn't valid in a case clause.

If we can get a more specific diagnostic, then we can add a fix to wrap it in the const ( and ) delimiters.