Skip to content

Misleading error message for ?? throw. #24891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lrhn opened this issue Nov 12, 2015 · 8 comments
Open

Misleading error message for ?? throw. #24891

lrhn opened this issue Nov 12, 2015 · 8 comments
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@lrhn
Copy link
Member

lrhn commented Nov 12, 2015

The expression

  x ?? throw new ArgumentError("NOooooooooooo....!");

gives the error message:

Error: Expected an expression, but got 'throw'.                                 
  x ?? throw new ArgumentError("NOooooooooooo....!");                           
       ^^^^^                                                                    
Error: Compilation failed.                                                      

This is misleading since throw is an expression. A more correct error message would be:

"throw" is not allowed at this point. Try wrapping the "throw" expression in parentheses.
@zoechi
Copy link
Contributor

zoechi commented Nov 12, 2015

"Try wrapping" sounds weird, like try a few things until you stumble upon something that doesn't produce an error.
Why is throw not allowed? What does wrapping in parentheses change?

@lrhn
Copy link
Member Author

lrhn commented Nov 12, 2015

Agree on wording. It's a suggestion, so it should't dictate that you do it, but "Maybe you want to wrap ..." is better than "Try wrapping".

The grammar production for ?? is logicalOrExpression '??' logicalOrExpression. A throwExpression is not reachable from a logicalOrExpression. That means that x ?? throw 0 is not valid Dart syntax - the grammar productions do not allow it. Wrapping throw 0 in parentheses changes it from being a throwExpression to being a plain expression which is reachable from logicalOrExpression.

It still sucks. I think the ?? operator should have the same precedence as the ?: operator, but even that would not help here because the throw operator binds very weakly (which was really a mistake and you can blame me for it).

@zoechi
Copy link
Contributor

zoechi commented Nov 12, 2015

"Maybe you want..." is definitely better, or "Wrapping the throw expression in parentheses would create an allowed expression."

@zoechi
Copy link
Contributor

zoechi commented Nov 12, 2015

@lrhn You say that ?? should have a different precedence and throw binds weakly.
Are there plans to fix this in Dart 2.0? Do you maintain a list of such things that should be fixed?
No need to publish it (would be nice though ;-) ), just want to know if you keep track of these things.

@lrhn
Copy link
Member Author

lrhn commented Nov 12, 2015

Not saying anything about ??, it's probably fine as-is - not exactly the same precedence as ?: (which is hard to talk about the precedence of anyway since it differs between the three operands), but it's right above it in the precedence table which is also fine, and possibly even better. That way x ?? y ? z ?? v : w ?? q isn't ambiguous.

I'm saying that throw binds weakly, and it shouldn't have, but it's a limited problem because I don't think most uses of throw depends on the precedence (who writes throw 3 + 4 anyway?).

So, no current plans to change it that I'm aware of.

I don't think x ?? throw ... is particularly recommendable code, and I'd request anyone writing to go back to if (x == null) throw ...;. Don't use an expression to do a statement's job. If you don't want the value from the expression, don't use an expression.

@zoechi
Copy link
Contributor

zoechi commented Nov 12, 2015

I see, thanks for your explanation 👍

@sigmundch sigmundch added the P3 A lower priority bug or feature request label Dec 9, 2015
@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed type-enhancement labels Mar 1, 2016
@askeksa-google askeksa-google added legacy-area-front-end Legacy: Use area-dart-model instead. and removed web-dart2js labels Jun 22, 2018
@askeksa-google
Copy link

This is a good suggestion which is still relevant.

@eernstg
Copy link
Member

eernstg commented Nov 21, 2019

Also note that we could allow e1 ?? throw e2, cf. dart-lang/language#644 (comment). If it's not an error then the choice of error message is less important. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
legacy-area-front-end Legacy: Use area-dart-model instead. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

6 participants