Diagnose precedence inversion in a warning wave #46239
Conversation
|
@dotnet/roslyn-compiler May I please have a couple of reviews of this change to move a diagnostic from "strict" mode to a warning wave? #Closed |
1 similar comment
|
@dotnet/roslyn-compiler May I please have a couple of reviews of this change to move a diagnostic from "strict" mode to a warning wave? #Closed |
| @@ -6340,6 +6340,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ | |||
| <data name="ERR_NonPrivateAPIInRecord" xml:space="preserve"> | |||
| <value>Record member '{0}' must be private.</value> | |||
| </data> | |||
| <data name="WRN_PrecedenceInversion" xml:space="preserve"> | |||
| <value>Operator cannot be used here due to precedence. Use parentheses to disambiguate.</value> | |||
There was a problem hiding this comment.
Consider including the operator token in the diagnostic message, e.g. "Operator 'as' cannot be used here due to precedence.." #Resolved
|
@dotnet/roslyn-compiler May I please have a second review? #Closed |
1 similar comment
|
@dotnet/roslyn-compiler May I please have a second review? #Closed |
| if (src.selectCount == 1) | ||
| errCount++; | ||
|
|
||
| Console.Write(errCount); | ||
| return (errCount > 0) ? 1 : 0; | ||
| } | ||
| }"; | ||
| // the grammar does not allow a query on the right-hand-side of &&, but we allow it except in strict mode. | ||
| CreateCompilationWithMscorlib40AndSystemCore(source, parseOptions: TestOptions.Regular.WithStrictFeature()).VerifyDiagnostics( |
There was a problem hiding this comment.
CreateCompilationWithMscorlib40AndSystemCore [](start = 12, length = 44)
We should continue to test this case. #Resolved
There was a problem hiding this comment.
I'm not sure what you mean. Strict more no longer diagnoses this. Are you suggesting that we should show the warning-wave warnings in this test?
In reply to: 463293894 [](ancestors = 463293894)
There was a problem hiding this comment.
Are you suggesting that we should show the warning-wave warnings in this test?
Yes, sorry, that wasn't clear in my earlier statement.
In reply to: 463310390 [](ancestors = 463310390,463293894)
| @@ -6352,6 +6352,12 @@ To remove the warning, you can use /reference instead (set the Embed Interop Typ | |||
| <data name="ERR_NonPrivateAPIInRecord" xml:space="preserve"> | |||
| <value>Record member '{0}' must be private.</value> | |||
| </data> | |||
| <data name="WRN_PrecedenceInversion" xml:space="preserve"> | |||
| <value>Operator '{0}' cannot be used here due to precedence. Use parentheses to disambiguate.</value> | |||
There was a problem hiding this comment.
Operator '{0}' cannot be used here due to precedence [](start = 11, length = 52)
This is a warning so it seems confusing that the message is "Operator cannot be used here due to precedence." We're allowing the operator so presumably it can be used. Should the message be more specific? Perhaps "Operator cannot be used here with the usual precedence." #ByDesign
There was a problem hiding this comment.
How would "usual" add any clarity? We're accepting the operator because we're not paying any attention to precedence at all.
In reply to: 463301820 [](ancestors = 463301820)
| // with an anonymous method expression or a lambda expression with a block body. No | ||
| // further parsing will find a way to fix things up, so we accept the operator but issue | ||
| // a diagnostic. | ||
| ErrorCode errorCode = leftOperand.Kind == SyntaxKind.IsPatternExpression ? ErrorCode.ERR_UnexpectedToken : ErrorCode.WRN_PrecedenceInversion; |
There was a problem hiding this comment.
ErrorCode.ERR_UnexpectedToken [](start = 95, length = 29)
Aren't we reporting an error in the non-pattern case now, where previously there was only an error if using "/strict"? #Resolved
There was a problem hiding this comment.
No, it is a warning : ErrorCode.WRN_PrecedenceInversion
In reply to: 463304547 [](ancestors = 463304547)
ghost
modified the milestones:
* upstream/master: (304 commits) Tweak diagnostics to account for records (dotnet#46341) Diagnose precedence inversion in a warning wave (dotnet#46239) Remove PROTOTYPE tag (dotnet#45965) Only run a single pass of NullableWalker per-member (dotnet#46402) Fix crash, and offer "declare as nullable" for tuple fields (dotnet#46437) Simplify contract for RunWithShutdownBlockAsync Fix optprof plugin input check if EditorAdaptersFactoryService gives us a null buffer Cannot assign maybe-null value to TNotNull variable (dotnet#41445) Fix overload resolution to handle binary compat in the face of covariant returns (dotnet#46367) Same failure on Linux Skip some tests on Mac Added search option for inline parameter name hints Spelling tweak docs Improve comment Improve the "not exhaustive" diagnostic in the presence of a when clause. (dotnet#46143) PR feedback Use record keyword to display records (dotnet#46338) remove test that aserts .NET Standard should be prefered over .NET Framework ...
Fixes #30231