-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Mark CS4009 error as a "build only" error. #80698
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
Conversation
|
@dotnet/roslyn-compiler ptal. |
|
I'd think I'd rather see us call |
|
Also, please don't forget to test VB here, I assume something similar may exist. |
I will be fine if we start with making a simple change that reflects the current state of affairs with respect to when the error is reported. Changing at which compilation phase the error is reported could be done later if we have a strong case for that. |
|
@AlekseyTs I'd argue that the original bug report is that case; @svick is confused why the error isn't showing up immediately. If that's not enough of a justification, then I don't know what would ever be enough of a justification. |
|
my concerns with GetEntryPointAndDiagnostics are:
|
|
Since the PR provided no explanation other than claiming that it fixed the issue, I assumed this change led to a desired IDE behavior. @CyrusNajmabadi could you please clarify? |
Yes. It addresses things where build errors are not filtered out when you go to 'build + intellisense' errors. Right now they are filtered out because we think this is a 'live' diagnostic, and thus we don't show the actual build diagnostic sincce we think it would cause duplicate errors. |
Sorry, it is still not clear to me. In your opinion, will the user remain confused after the change or not? |
|
I do not believe they will be confused. |
|
@dotnet/roslyn-compiler for eyes. |
I do; reading @svick's original report, my expectation is that they're expecting this error to be live reported, which this PR does not do. However, I am also sympathetic to the idea that we cannot determine this incrementally in a single file in a performant fashion, and that it may simply not be possible to do this in a satisfactory fashion. Unrelated: @CyrusNajmabadi can you please update the PR title to be more descriptive? |
|
Dismissed my review; @CyrusNajmabadi, can you please see if VB suffers from the same issue, and either confirm it does not or update accordingly? |
|
@copilot can you make all the same corresponding changes for VB for ERR_AsyncSubMain |
* upstream/main: (332 commits) Cache lambdas in analyzer driver (dotnet#80759) Add information for NuGet package version 4.14 (dotnet#80870) Add missing search keywords to VB Advanced options page Fix IDE0031 false positive when preprocessor directives are used in if statements (dotnet#80878) Use core compiler on netfx hosts with toolset package (dotnet#80631) Make string concat assert more precise (dotnet#80619) Extensions: address some diagnostic quality issues (dotnet#80827) Add note on traversal order for bound nodes (dotnet#80872) Ensure that locals at the top level of a constructor have the same safe-context as parameters (dotnet#80807) Fix handling of SymbolDisplayCompilerInternalOptions.UseArityForGenericTypes option for non-native symbol implementations (dotnet#80826) Update src/Analyzers/CSharp/Tests/UseCollectionInitializer/UseCollectionInitializerTests.cs Add IsValidContainingStatement check to prevent collection initializers in using declarations Add back old DocumentSpan constructor (dotnet#80864) Add tests verifying pointer types in type parameters require unsafe context (dotnet#80776) Add regression test for Interlocked.Exchange with nullable types (dotnet#80796) Add regression test for ParseAttributeArgumentList with invalid input (fixes dotnet#8699) (dotnet#80705) Add regression test for compiler crash with syntax error in indexer declaration (dotnet#80772) Add runtime NullReferenceException validation to foreach null iteration tests (dotnet#80839) Update MicrosoftBuildTasksCoreVersionForMetrics to 17.11.48 (dotnet#80812) Mark CS4009 error as a "build only" error. (dotnet#80698) ...
Fixes #17259