Fix attribute resolution in recursive module/namespace scopes#19744
Draft
T-Gro wants to merge 9 commits into
Draft
Fix attribute resolution in recursive module/namespace scopes#19744T-Gro wants to merge 9 commits into
T-Gro wants to merge 9 commits into
Conversation
Adds AttributeResolutionInRecursiveScopes.fs with 4 baseline regression tests covering attribute positions that already work today (module rec, namespace rec, non-rec module). Subsequent sprints will append tests for the broken cases from issue #5795. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
In module/namespace rec, attributes on union cases couldn't reference attribute types defined in the same recursive group because TcUnionCaseDecl ran eager TcAttributes during Phase1G before attribute constructors were wired up. Mirror the Phase1B pattern used for tycon attributes: use TcAttributesCanFail to get preliminary attrs plus a deferred getFinalAttrs thunk, and run the thunk via fixupFinalAttrs after all Phase1G representations are established. Fixes #5795 (union case case). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
In module/namespace rec, attributes on record fields couldn't reference attribute types defined in the same recursive group because TcFieldDecl ran eager TcAttributesWithPossibleTargets during Phase1G before attribute constructors were wired up. Add TcAttributesWithPossibleTargetsCanFail (mirrors TcAttributesCanFail) and use it in TcFieldDecl. TcFieldDecl now returns the RecdField plus a fixup thunk that re-resolves attributes after Phase1G completes. Field fixups are plumbed through TcAnonFieldDecl, TcNamedFieldDecl, TcNamedFieldDecls into the Phase1G representation handlers for record and class/struct types. Union case TcUnionCaseDecl composes its own field fixups into the union-case attribute fixup added in sprint 02. Exception field attrs are resolved eagerly to preserve current behavior (they are not part of #5795). The Phase1G orchestration variable is renamed fixupUnionCaseAttrs -> fixupReprAttrs to reflect that it now covers both union case and record/class field attribute fixups. Fixes #5795 (record field case). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
In module/namespace rec, attributes on generic type parameters couldn't reference attribute types defined in the same recursive group because TcTyparDecl ran eager TcAttributes during Phase1A_BuildInitialTycon, before any rec-scope tycon had been published to the env. Resolve attributes in a preliminary pass with error reporting suppressed so user-defined attributes from the same rec group don't prematurely emit FS0039. Framework attributes (Measure, EqualityConditionalOn, ComparisonConditionalOn, CompiledName) are always in scope and resolve eagerly, preserving kind inference and conditional-dependency flags. A per-typar fixup thunk re-resolves attributes against a richer env and finalizes the typar attribs via Typar.SetAttribs. For non-rec callers, the fixup runs immediately with the same env; for the Phase1A path, it is composed into Phase1G's per-tycon fixupFinalAttrs, keyed by Tycon stamp. Fixes #5795 (typar case, final compiler change). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…c scopes Adds 10 tests covering nested-module attribute resolution, attribute shadowing, mixed framework+custom attributes, and negative cases (unknown names, non-attribute types). The MeasureAttribute shadowing test is skipped because F# kind inference is name-resolution based and the scenario is unrelated to the #5795 fix. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…otes Add the missing namespace-rec union-case attribute test (sprint 02 only covered the module rec variant) so all 16 PLAN cases are represented. Add release-notes bullet for the fix under docs/release-notes/.FSharp.Compiler.Service/11.0.100.md. Validation: - ./build.sh -c Release: 0 warnings, 0 errors - ComponentTests: 7076 passed, 0 failed - Service.Tests: 2163 passed, 0 failed (incl. SurfaceArea) - FSharpSuite/IDE/Core: all green - Issue #5795 repros (type/let/case/typar) all compile clean Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n recoverable exn Address two issues flagged by CODE-QUALITY and HONEST-ASSESSMENT reviewers on the #5795 fix: 1. TcTyparDecl (CheckExpressions.fs ~4518): previously re-ran TcAttributes unconditionally in the fixup thunk, doubling CallNameResolutionSink calls and attribute-arg typechecking for every attributed typar in every project. Now matches the sibling TcAttributesCanFail / TcAttributesWithPossibleTargetsCanFail pattern: gate the re-resolution on a 'didFail' signal derived from - TcAttributesMaybeFail's reported didFail, - any synAttr dropped by the inner RecoverableException catch (length mismatch), - any diagnostic captured by a scoped CapturingDiagnosticsLogger during the suppressed prelim (covers warnings like FS0842, which the previous gated-only pattern would silently swallow). When prelim resolves cleanly, the fixup is a no-op. 2. TcTyconDefnCore_Phase1G_EstablishRepresentation (CheckDeclarations.fs ~3424): the RecoverableException recovery path returned (fun () -> ()) as the deferred attribute fixup, overwriting any fixupReprAttrs already captured by Union/Record/General arms. A latestFixupReprAttrs ref now lives outside the try and is updated alongside the inner mutable; the handler returns its captured value so rec-resolved field/case attribute diagnostics survive a later recoverable error. Validation: - dotnet build src/Compiler/FSharp.Compiler.Service.fsproj -c Release: clean - ComponentTests: 7074 passed, 261 skipped, 2 pre-existing failures (Thai locale, sequence-handler test) confirmed unrelated on baseline. - Sprint-23 + skipped test: 23 passed, 1 skipped (unchanged). - SurfaceArea: passes (no public API change). - Service.Tests: 4 pre-existing type-provider failures (3/4 also fail on baseline); no new regressions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
❗ Release notes required
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
T-Gro
added
AI-Auto-Resolve-Conflicts
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5795
Attributes defined in a
recmodule or namespace can now be used on types, union cases, record fields, and type parameters within the same recursive scope.