Skip to content

Conversation

@CyrusNajmabadi
Copy link
Member

Fixes #17259

@CyrusNajmabadi
Copy link
Member Author

@dotnet/roslyn-compiler ptal.

@333fred
Copy link
Member

333fred commented Oct 14, 2025

I'd think I'd rather see us call GetEntryPointAndDiagnostics from GetDiagnostics, rather than marking this as a build-only error.

@333fred
Copy link
Member

333fred commented Oct 14, 2025

Also, please don't forget to test VB here, I assume something similar may exist.

@AlekseyTs
Copy link
Contributor

I'd think I'd rather see us call GetEntryPointAndDiagnostics from GetDiagnostics, rather than marking this as a build-only error.

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.

@333fred
Copy link
Member

333fred commented Oct 14, 2025

@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.

@CyrusNajmabadi
Copy link
Member Author

my concerns with GetEntryPointAndDiagnostics are:

  1. is it expensive? i thought it would have to search the entire compilation
  2. can it be called for the diagnostics for a particular file? i would think this was a question asked of the whole compilation.

@333fred
Copy link
Member

333fred commented Oct 14, 2025

  1. Potentially, but since GetDiagnostics binds everything anyway, I don't really expect it to be that different.
  2. I don't think so, no, it would only be whole project.

@AlekseyTs
Copy link
Contributor

If that's not enough of a justification, then I don't know what would ever be enough of a justification.

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?

@CyrusNajmabadi
Copy link
Member Author

Since the PR provided no explanation other than claiming that it fixed the issue, I assumed this change led to a desired IDE behavior.

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.

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Oct 14, 2025

Since the PR provided no explanation other than claiming that it fixed the issue, I assumed this change led to a desired IDE behavior.

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?

@CyrusNajmabadi
Copy link
Member Author

I do not believe they will be confused.

@CyrusNajmabadi
Copy link
Member Author

@dotnet/roslyn-compiler for eyes.

@333fred
Copy link
Member

333fred commented Oct 20, 2025

I do not believe they will be confused.

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?

@CyrusNajmabadi CyrusNajmabadi changed the title Mark error as build only Mark CS4009 error as a "build only" error. Oct 20, 2025
333fred
333fred previously approved these changes Oct 20, 2025
@333fred 333fred dismissed their stale review October 20, 2025 17:04

Still need VB testing

@333fred
Copy link
Member

333fred commented Oct 20, 2025

Dismissed my review; @CyrusNajmabadi, can you please see if VB suffers from the same issue, and either confirm it does not or update accordingly?

@CyrusNajmabadi
Copy link
Member Author

@copilot can you make all the same corresponding changes for VB for ERR_AsyncSubMain

@CyrusNajmabadi CyrusNajmabadi merged commit 6c787e4 into dotnet:main Oct 23, 2025
28 checks passed
@CyrusNajmabadi CyrusNajmabadi deleted the buildOnlyError branch October 23, 2025 10:22
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Oct 23, 2025
333fred added a commit to 333fred/roslyn that referenced this pull request Oct 24, 2025
* 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)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"Entry point cannot be marked async" error not showing in Build + IntelliSense Error List

6 participants