6.0.400 compilation error with private types in namespace used in a different file
Repro steps
Defining a private type in a namespace and using it in a module in another file in the same namespace leads to a compilation error in 6.0.400.
Compiles without error in 6.0.300.
A simple repro can be found here: https://github.com/l3m/fsharp-error-repro
Expected behavior
Unclear if the behavior in 6.0.300 or 6.0.400 is the actually correct one, but it's certainly a change.
So the expectation would have been that it compiles in 6.0.400
Actual behavior
Fails to compile in 6.0.400
Known workarounds
Replace
type private Blah = ...
with
type internal Blah = ...
Related information
Observed on both linux and windows, with dotnet sdk 6.0.300 and 6.0.400
Looks like this suggestion been fixed https://github.com/fsharp/fslang-suggestions/issues/389 Let me bisect and find where it was changed.
Ok, with help of @0101, we bisected to that PR: https://github.com/dotnet/fsharp/pull/13287 CC @nojaf @dsyme
@vzarytovskii I can't really put my finger on it.
For some reason the Blah type is not being added to https://github.com/dotnet/fsharp/blob/bdb64624f0ca220ca4433c83d02dd5822fe767a5/src/Compiler/Checking/NameResolution.fsi#L215
and it is later not found in
https://github.com/dotnet/fsharp/blob/bdb64624f0ca220ca4433c83d02dd5822fe767a5/src/Compiler/Checking/NameResolution.fs#L3311
The problem appears to go away with a CustomEquality for type ModuleOrNamespaceKind.
[<CustomEquality; NoComparison>]
type ModuleOrNamespaceKind =
/// Indicates that a module is compiled to a class with the "Module" suffix added.
| FSharpModuleWithSuffix
/// Indicates that a module is compiled to a class with the same name as the original module
| ModuleOrType
/// Indicates that a 'module' is really a namespace
| Namespace of
/// Indicates that the sourcecode had a namespace.
/// If false, this namespace was implicitly constructed during type checking.
isExplicit: bool
override this.Equals other =
match other with
| :? ModuleOrNamespaceKind as kind ->
match this, kind with
| FSharpModuleWithSuffix, FSharpModuleWithSuffix
| ModuleOrType, ModuleOrType
| Namespace _, Namespace _ -> true
| _ -> false
| _ -> false
override this.GetHashCode () =
match this with
| FSharpModuleWithSuffix -> 0
| ModuleOrType -> 1
| Namespace _ -> 2
@vzarytovskii
Huh, interesting. Do did lookup work before?
I could be way off here but I think some structure that contains a ModuleOrNamespaceKind is being compared somewhere.
For example: https://github.com/dotnet/fsharp/blob/bdb64624f0ca220ca4433c83d02dd5822fe767a5/src/Compiler/TypedTree/TypedTree.fs#L503-L528
Subtle introduction of the bug. Marking CompilationPath as NoEquality may help track track this down, looks like it is the structural equality on this:
[<StructuralEquality; NoComparison; StructuredFormatDisplay("{DebugText}")>]
type Accessibility =
| TAccess of compilationPaths: CompilationPath list
By using this diff https://github.com/dotnet/fsharp/compare/main...dsyme:cwe?expand=1 it looks like these are the origin points for structural equality
let isPublicVal (lv: Val) = (lv.Accessibility = taccessPublic)
let isPublicUnionCase (ucr: UnionCaseRef) = (ucr.UnionCase.Accessibility = taccessPublic)
let isPublicRecdField (rfr: RecdFieldRef) = (rfr.RecdField.Accessibility = taccessPublic)
let isPublicTycon (tcref: Tycon) = (tcref.Accessibility = taccessPublic)
Also this in AccessibilityLogic:
| AccessibleFrom(cs1, tc1), AccessibleFrom(cs2, tc2) -> (cs1 = cs2) && (match tc1, tc2 with None, None -> true | Some tc1, Some tc2 -> tyconRefEq g tc1 tc2 | _ -> false)
One option is to switch to NoEquality and NoComparison on Accessibility and CompilationPath and use an explicit equality routine in the above places.
Another is to use the custom equality like @nojaf above, though I think perhaps just making everything more explicit is better. There are enough notions of equality needed in the compiler its better to be explicit for anything in TypedTree
This particular case seems to be caused by the comparison made here: https://github.com/dotnet/fsharp/blob/5420f094882d1e16e970727f7474ce06cbe57b5a/src/Compiler/TypedTree/TypedTreeBasics.fs#L443-L450
But I imagine it could also cause a similar issue here: https://github.com/dotnet/fsharp/blob/5420f094882d1e16e970727f7474ce06cbe57b5a/src/Compiler/Symbols/Symbols.fs#L125-L132
Switching Accessibility and CompilationPath to NoEquality wouldn't really force the fix here since the equality comparison is performed directly on ModuleOrNamespaceKind.
We can mark that one as NoEquality but that will add quite a bit of extra equality code in different places. So I would lean more towards @nojaf 's custom equality there. Especially since I don't see a need for the current default version where a implicitly defined namespace is considered different than the same but explicitly defined one (though I might be wrong there).
Is there a chance we could get this fix rolled out soon? The SDK version 6.0.401 (released yesterday) is still failing to build our project even though the fix was merged one month ago.
Is there a chance we could get this fix rolled out soon? The SDK version 6.0.401 (released yesterday) is still failing to build our project even though the fix was merged one month ago.
Fix was only merged to .NET7 branch, we couldn't do servicing to 401, it was too late. One of future 6.x.x servicing is a target.
Ok, thanks for the quick response. We'll have to hold out another month or two then. :'(
Ok, thanks for the quick response. We'll have to hold out another month or two then. :'(
Yeah, sorry for that, was a bit unfortunate timing on our side.