dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.82k stars 773 forks source link

GraphBasedChecking fails when exceptions are defined #17262

Closed Smaug123 closed 1 month ago

Smaug123 commented 1 month ago

Enabling GraphBasedChecking can cause compilation to fail.

Repro steps

Create Repro.fsproj:

<Project Sdk="Microsoft.NET.Sdk">

    <PropertyGroup>
        <TargetFramework>net8.0</TargetFramework>
        <OtherFlags>$(OtherFlags) --test:GraphBasedChecking --warnaserror-:75 --nowarn:75</OtherFlags>
    </PropertyGroup>

    <ItemGroup>
        <Compile Include="Blah.fs" />
        <Compile Include="Program.fs"/>
    </ItemGroup>

</Project>

Create Blah.fs:

namespace Foo

exception internal Blah

Create Program.fs:

namespace Foo

module Program =

    [<EntryPoint>]
    let main _ =
        raise Blah
        0

Expected behavior

Build succeeds.

Actual behavior

0>Program.fs(7,15): Error FS0039 : The value or constructor 'Blah' is not defined. 0>------- Finished building project: Repro. Succeeded: False. Errors: 1. Warnings: 0

Known workarounds

Disable GraphBasedChecking.

Related information

Provide any related information (optional):

dotnet --info                                                                                                                                                                                                                ~/Documents/GitHub/Repro
.NET SDK:
 Version:           8.0.300
 Commit:            326f6e68b2
 Workload version:  8.0.300-manifests.c1c70047
 MSBuild version:   17.10.4+10fbfbf2e

Runtime Environment:
 OS Name:     Mac OS X
 OS Version:  14.5
 OS Platform: Darwin
 RID:         osx-arm64
 Base Path:   /nix/store/cjfvjcg710nrihrcprxfa9rnxgdy2d24-dotnet-sdk-8.0.300/sdk/8.0.300/

.NET workloads installed:
There are no installed workloads to display.

Host:
  Version:      8.0.5
  Architecture: arm64
  Commit:       087e15321b

.NET SDKs installed:
  8.0.300 [/nix/store/cjfvjcg710nrihrcprxfa9rnxgdy2d24-dotnet-sdk-8.0.300/sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.App 8.0.5 [/nix/store/cjfvjcg710nrihrcprxfa9rnxgdy2d24-dotnet-sdk-8.0.300/shared/Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 8.0.5 [/nix/store/cjfvjcg710nrihrcprxfa9rnxgdy2d24-dotnet-sdk-8.0.300/shared/Microsoft.NETCore.App]

Other architectures found:
  None

Environment variables:
  Not set

global.json file:
  Not found

Learn more:
  https://aka.ms/dotnet/info

Download .NET:
  https://aka.ms/dotnet/download
vzarytovskii commented 1 month ago

@safesparrow @nojaf any pointers where I could look at? If not, I'll debug it later this week or next week

majocha commented 1 month ago

Same error can be seen in IDE with Transparent Compiler enabled.

majocha commented 1 month ago

Exception's identifier "Blah" is held by SynUnionCase:

      SynUnionCase.SynUnionCase(
          attributes = [],
          ident = SynIdent.SynIdent(ident = Ident("Blah", R("(3,10--3,14)")), trivia = None),
          caseType = SynUnionCaseKind.Fields([]),
          xmlDoc = PreXmlDoc.Empty,
          accessibility = None,
          range = R("(3,10--3,14)"),
          trivia = { BarRange = None }

but visitSynUnionCase does not yield anything here because it does not handle the ident field: https://github.com/dotnet/fsharp/blob/7e593275025946f062c83fe75c705f196ac532bc/src/Compiler/Driver/GraphChecking/FileContentMapping.fs#L97-L103

So possibly the graph checking misses that dependency?

majocha commented 1 month ago

I tried to run the repro in debug. It seems we do try to emit FileContentEntry.PrefixedIdentifier for the exception Blah but for some reason longId field here is None: https://github.com/dotnet/fsharp/blob/7e593275025946f062c83fe75c705f196ac532bc/src/Compiler/Driver/GraphChecking/FileContentMapping.fs#L64-L70

nojaf commented 1 month ago

I might know what the problem is here, will take a look later this week.

nojaf commented 1 month ago

A workaround for now could be to express the exceptions are types:

type internal Blah() =
    inherit System.Exception()