Skip to content

Conversation

@auduchinok
Copy link
Member

@auduchinok auduchinok commented Jan 30, 2019

It seems storing the name is needed only as a workaround for opNameCons and opNameNil cases and can be computed from Id then same way when needed instead.

@auduchinok auduchinok changed the title [WIP] Do not store union case compiled name Do not store union case compiled name Jan 30, 2019
@auduchinok auduchinok force-pushed the unionCase-compiledName branch from 9079e3b to 0e0c5af Compare January 30, 2019 14:33
@alfonsogarciacaro
Copy link
Contributor

Is the CompiledNameAttribute actually used in union cases? We use it in multiple occasions in Fable, but there doesn't seem to be a way to get it through reflection in .NET and we need to check the attribute manually.

@auduchinok
Copy link
Member Author

auduchinok commented Jan 30, 2019

@alfonsogarciacaro No, they're ignored, at least currently.
Details of compiled forms and names of union cases are described in 8.5.4 of the F# 4.1 spec.
13.1 chapter also tells where attributes are placed for union cases in compiled form, making CompiledName case look a bit weird, although funny:

type U =
    | [<CompiledName("AA")>] A of int
[CompiledName("AA")]
[CompilationMapping]
public static Module.U NewA(int item)
{
  return new Module.U(item);
}

The spec says following for the attribute itself:

Changes the compiled name of an F# language construct. This attribute should be used only in F# assemblies.

So it could override names coming from 8.5.4, but I'm not sure it's ever going to change.

@auduchinok
Copy link
Member Author

It's ready. I've changed error ranges for the following error to only contain case names and not full case ranges:

The union case named 'Tags' conflicts with the generated type 'Tags'

@TIHan TIHan requested a review from dsyme January 30, 2019 20:52
@KevinRansom KevinRansom merged commit 5a5ca97 into dotnet:master Jan 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants