[JEP 488][Preview][Primitive Patterns] ECJ crashes compiling switch over void typed selector expression #3722
Conversation
| " switch (foo()) {\n" + | ||
| " ^^^^^\n" + | ||
| "Expression must return a value\n" + | ||
| "----------\n"); |
There was a problem hiding this comment.
- Are both arms of the ternary identical now ? They appear so. Could they be collapsed together ?
- With the message
Expression must return a valueI feel the quality of the diagnostic actually degrades in this context. How about we change the message toExpression has no value- ("to switch on", "to synchronize with" etc left to be understood) ?
There was a problem hiding this comment.
ad (1): your eagle eyes spotted correctly :)
There was a problem hiding this comment.
ad (2): I agree, that message is a bit odd.
"has no value" makes me think of "worthless" :)
Perhaps we can agree on "yields no value"?
Let me see if that change will cause a landslide in our tests.
There was a problem hiding this comment.
Let me see if that change will cause a landslide in our tests.
Quite to the contrary. I can't find any tests covering the 2 pre-existing uses of this error message. I'll add a couple.
| if (expressionType.id == TypeIds.T_void) { | ||
| upperScope.problemReporter().illegalVoidExpression(this.expression); | ||
| break checkType; | ||
| } |
There was a problem hiding this comment.
Admittedly, this is even more corner case: What about 'V'oid methods ? should they be handled here too ? (Actually what can a Void method return other than null ? I have poor understanding here)
There was a problem hiding this comment.
Indeed I can't see how switching over Void could ever do anything useful (you'd get NPE at best). Yet doing so would be legal code, in contrast to void.
void typed selector expression + report error rather than crashing Fixes eclipse-jdt#3369
void typed selector expression + advoid secondary error + tentatively adjust tests from eclipse-jdt#2385 Fixes eclipse-jdt#3369
void typed selector expression + rephrase error message + add tests for pre-existing cases of that error Fixes eclipse-jdt#3369
stephan-herrmann
force-pushed
the
issue3369
branch
from
c87d1e2 to
01543cb
Compare
Fixes #3369