Skip to content

Show [<Literal>] and constant value for IL fields in Go to Metadata - #19922

Merged
T-Gro merged 10 commits into
mainfrom
fix/issue-11526
Jul 7, 2026
Merged

Show [<Literal>] and constant value for IL fields in Go to Metadata#19922
T-Gro merged 10 commits into
mainfrom
fix/issue-11526

Conversation

@T-Gro

@T-GroT-Gro commented Jun 9, 2026

Copy link
Copy Markdown
Member

Fixes#11526

IL literal (const) fields such as System.Char.MaxValue now render with
[<Literal>] and their constant value in Go to Metadata, instead of
appearing as plain static val declarations.

Copilotand others added 3 commits June 9, 2026 08:13
…11526)
Adds a new MetadataAsTextILField module in
tests/FSharp.Compiler.Service.Tests/Symbols.fs covering the rendering of
IL literal (const) static fields via FSharpEntity.TryGetMetadataText().
Tests added:
- Theory with 5 rows (Int32/Int64/Byte/SByte MaxValue/MinValue) asserting
the rendered metadata contains [<Literal>] and "= <value>".
- Fact for System.Char.MaxValue asserting [<Literal>] and "=" appear on
the MaxValue line.
- Negative Fact for System.Math.PI (initonly, non-literal) asserting it
is NOT tagged [<Literal>] and has no "=".
Current state (RED):
total: 7, failed: 6, succeeded: 1
- 5 Theory rows + Char.MaxValue Fact fail with
Assert.Contains() Failure: Sub-string not found
because NicePrint.layoutILFieldInfo currently drops [<Literal>] and
the literal value.
- Math.PI passes (guards against regression of normal static val
rendering).
Sprint 02 will implement the fix in src/Compiler/Checking/NicePrint.fs.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…-to-Metadata
Note: Sprint 01 test assumptions were partially incorrect:
- Math.PI is actually a literal const in IL (not initonly); replaced with System.String.Empty for the non-literal case.
- The 'first line containing fieldName' finder matched doc-comment lines; narrowed to lines containing 'val <fieldName>:'.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
fantomas no-op (both touched files are in .fantomasignore); no .bsl baseline drift observed in targeted MetadataAsTextILField run (7/7 passed in 13s).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions

github-actionsBot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change pathRelease notes pathDescription
src/Compilerdocs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@github-actionsgithub-actionsBot added the AI-Tooling-Check-Bypassed Tooling check: non-fork PR, not diff-analyzed label Jun 9, 2026
CI `check_release_notes` requires the entry to live in the VNEXT release-notes path (currently 11.0.100.md), not in the shipped 9.0.300.md release. Move the bullet, no behavior change.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro
T-Gro requested a review from abonieJune 12, 2026 10:08

@T-GroT-Gro left a comment

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 This review was generated by AI (@expert-reviewer agent). Findings may contain inaccuracies — please verify independently.

Reviewed the IL literal field rendering change in NicePrint.fs and the accompanying tests. The core approach (append the init value and the [<Literal>] attribute, gated on finfo.LiteralValue.IsSome, reusing the existing layoutAttribs/layoutILFieldInit helpers) is sound and consistent with how F# literal vals are already printed. One substantive gap noted inline regarding string/null literal fields.

Comment threadsrc/Compiler/Checking/NicePrint.fs
@T-GroT-Gro added the AI-reviewed PR reviewed by AI review council label Jun 25, 2026

@abonieabonie left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with the comment about ILFieldInit.String/Null, otherwise PR looks good

@github-project-automationgithub-project-automationBot moved this from New to In Progress in F# Compiler and ToolingJun 25, 2026
Handle const string and null-literal IL fields in PrintIL.layoutILFieldInit so they emit the quoted string / null instead of '(* value unavailable *)'. The match is now exhaustive over ILFieldInit. Adds a string-literal test.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@T-Gro

Copy link
Copy Markdown
MemberAuthor

Addressed in 8b17938: PrintIL.layoutILFieldInit now handles ILFieldInit.String (emits the quoted string) and ILFieldInit.Null (emits
ull), so const string/null literal fields no longer render as (* value unavailable *). The match is now exhaustive over ILFieldInit (removed the _ -> None fallthrough). Added a string-literal test (RuntimeFeature.PortablePdb) to Symbols.fs. All MetadataAsTextILField tests pass.

@T-Gro
T-Gro requested a review from abonieJune 30, 2026 14:47
@T-Gro
T-Gro enabled auto-merge (squash) June 30, 2026 14:48
@T-Gro
T-Gro requested a review from abonieJuly 3, 2026 13:23
@T-Gro
T-Gro requested a review from abonieJuly 5, 2026 16:35
@T-Gro
T-Gro merged commit 19565c1 into mainJul 7, 2026
50 checks passed
@github-project-automationgithub-project-automationBot moved this from In Progress to Done in F# Compiler and ToolingJul 7, 2026
T-Gro added a commit that referenced this pull request Jul 8, 2026
* Emit FS0755 for CompiledName on multi-value let-bindings (#19924)
* Show [<Literal>] and constant value for IL fields in Go to Metadata (#19922)
* Fix shouldBeParenthesizedInContext for Sequential in record fields (#17826) (#19850)
* Migrate devdiv/dotnet-core-internal-tooling to WIF service connection (#20036)
Replace PAT-backed externalnugetfeed SC with WIF-based
azureDevOpsServiceConnection for the devdiv/dotnet-core-internal-tooling
NuGet feed. Part of PAT disable policy migration (WI 10124).
New SC: dnceng-devdiv-dotnet-core-internal-tooling-feed-rw-wif
* Make parallel and sequential code generation byte-identical (#19929)
* Update dependencies from https://github.com/dotnet/roslyn build 20260706.6 (#20037)
On relative base path root
Microsoft.CodeAnalysis , Microsoft.CodeAnalysis.Compilers , Microsoft.CodeAnalysis.CSharp , Microsoft.CodeAnalysis.EditorFeatures , Microsoft.CodeAnalysis.EditorFeatures.Text , Microsoft.CodeAnalysis.ExternalAccess.FSharp , Microsoft.CodeAnalysis.Features , Microsoft.VisualStudio.LanguageServices From Version 5.10.0-1.26352.10 -> To Version 5.10.0-1.26356.6
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Fix ppc64le stack overflow from deep Sequential chains in graph checking (#20028)
* Fix#13099: optimizer drops side-effectful receiver of unit member access in task CE (#19885)
* Reset files to feature/net11-scouting
Reset patterns:
- global.json
- eng/Version.Details.xml
- eng/Version.Details.props
- eng/Versions.props
- eng/common/**
- eng/TargetFrameworks.props
* Update dependencies from https://github.com/dotnet/roslyn build 20260707.6 (#20042)
On relative base path root
Microsoft.CodeAnalysis , Microsoft.CodeAnalysis.Compilers , Microsoft.CodeAnalysis.CSharp , Microsoft.CodeAnalysis.EditorFeatures , Microsoft.CodeAnalysis.EditorFeatures.Text , Microsoft.CodeAnalysis.ExternalAccess.FSharp , Microsoft.CodeAnalysis.Features , Microsoft.VisualStudio.LanguageServices From Version 5.10.0-1.26356.6 -> To Version 5.10.0-1.26357.6
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Update dependencies from https://github.com/dotnet/msbuild build 20260707.8 (#20041)
On relative base path root
Microsoft.Build , Microsoft.Build.Framework , Microsoft.Build.Tasks.Core , Microsoft.Build.Utilities.Core From Version 18.10.0-preview-26353-05 -> To Version 18.10.0-preview-26357-08
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
---------
Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com>
Co-authored-by: Missy Messa <47990216+missymessa@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>
T-Gro added a commit that referenced this pull request Jul 9, 2026
* Update dependencies from https://github.com/dotnet/arcade build 20260630.1
On relative base path root
Microsoft.DotNet.Arcade.Sdk From Version 11.0.0-beta.26325.1 -> To Version 11.0.0-beta.26330.1
* Emit FS0755 for CompiledName on multi-value let-bindings (#19924)
* Show [<Literal>] and constant value for IL fields in Go to Metadata (#19922)
* Fix shouldBeParenthesizedInContext for Sequential in record fields (#17826) (#19850)
* Migrate devdiv/dotnet-core-internal-tooling to WIF service connection (#20036)
Replace PAT-backed externalnugetfeed SC with WIF-based
azureDevOpsServiceConnection for the devdiv/dotnet-core-internal-tooling
NuGet feed. Part of PAT disable policy migration (WI 10124).
New SC: dnceng-devdiv-dotnet-core-internal-tooling-feed-rw-wif
* Make parallel and sequential code generation byte-identical (#19929)
* Update dependencies from https://github.com/dotnet/roslyn build 20260706.6 (#20037)
On relative base path root
Microsoft.CodeAnalysis , Microsoft.CodeAnalysis.Compilers , Microsoft.CodeAnalysis.CSharp , Microsoft.CodeAnalysis.EditorFeatures , Microsoft.CodeAnalysis.EditorFeatures.Text , Microsoft.CodeAnalysis.ExternalAccess.FSharp , Microsoft.CodeAnalysis.Features , Microsoft.VisualStudio.LanguageServices From Version 5.10.0-1.26352.10 -> To Version 5.10.0-1.26356.6
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
* Fix ppc64le stack overflow from deep Sequential chains in graph checking (#20028)
* Re-trigger CI (flaky FSharp.Compiler.Service.Tests hang)
The WindowsNoRealsig_testCoreclr leg failed because the FSharp.Compiler.Service.Tests (net11.0) assembly hung and was killed by the 5-minute hangdump (exit 57005), with no deterministic test failure (error:1, failed:0). This is unrelated to the arcade SDK bump, which only touches build infra. Empty commit to re-run CI.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix#13099: optimizer drops side-effectful receiver of unit member access in task CE (#19885)
---------
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Tomas Grosup <Tomas.Grosup@gmail.com>
Co-authored-by: Missy Messa <47990216+missymessa@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Copilot <copilot@github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-reviewedPR reviewed by AI review councilAI-Tooling-Check-BypassedTooling check: non-fork PR, not diff-analyzed

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Go to Metadata - static field handling is wrong

2 participants

@T-Gro@abonie