Skip to content

Commit

Permalink
Bugfix :: Nullness in signature file is not considered by implementat…
Browse files Browse the repository at this point in the history
…ion and vice versa (#18186)
  • Loading branch information
T-Gro authored Feb 6, 2025
1 parent 674e099 commit 841ba8e
Show file tree
Hide file tree
Showing 92 changed files with 639 additions and 323 deletions.
5 changes: 5 additions & 0 deletions .fantomasignore
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ src/Compiler/Checking/TypeRelations.fs

# nullness-related problems
src/Compiler/DependencyManager/DependencyProvider.fs
src/FSharp.Core/fslib-extra-pervasives.fs
src/FSharp.Core/fslib-extra-pervasives.fsi

# Incorrectly formatted: https://github.com/dotnet/fsharp/pull/14645/commits/49443a67ea8a17670c8a7c80c8bdf91f82231e91 or https://github.com/fsprojects/fantomas/issues/2733
# This CompilerImports.fs behavior is not fixed yet, following up in https://github.com/fsprojects/fantomas/issues/2733
Expand Down Expand Up @@ -124,6 +126,9 @@ src/Compiler/SyntaxTree/LexerStore.fs
src/Compiler/Driver/GraphChecking/Graph.fsi
src/Compiler/Driver/GraphChecking/Graph.fs

src/Compiler/DependencyManager/NativeDllResolveHandler.fsi
src/Compiler/DependencyManager/AssemblyResolveHandler.fsi

# Fantomas limitations on implementation files (to investigate)

src/Compiler/AbstractIL/ilwrite.fs
Expand Down
1 change: 1 addition & 0 deletions docs/release-notes/.FSharp.Compiler.Service/9.0.300.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
### Added
* Added missing type constraints in FCS. ([PR #18241](https://github.com/dotnet/fsharp/pull/18241))
* The 'use' keyword can be used on IDisposable|null without nullness warnings ([PR #18262](https://github.com/dotnet/fsharp/pull/18262))
* Nullness warnings are issued for signature<>implementation conformance ([PR #18186](https://github.com/dotnet/fsharp/pull/18186))
* Symbols: Add FSharpAssembly.IsFSharp ([PR #18290](https://github.com/dotnet/fsharp/pull/18290))

### Changed
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/AbstractIL/ilreflect.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2548,7 +2548,7 @@ let EmitDynamicAssemblyFragment
ignore (typB.InvokeMemberAndLog(methodName, BindingFlags.InvokeMethod ||| BindingFlags.Public ||| BindingFlags.Static, [||]))
None
with :? TargetInvocationException as exn ->
Some exn.InnerException
Option.ofObj exn.InnerException

let emEnv, entryPts = envPopEntryPts emEnv
let execs = List.map execEntryPtFun entryPts
Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/Checking/AttributeChecking.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ type AttribInfo =
| FSAttribInfo of TcGlobals * Attrib
| ILAttribInfo of TcGlobals * Import.ImportMap * ILScopeRef * ILAttribute * range

member ConstructorArguments: (TType * obj) list
member NamedArguments: (TType * string * bool * obj) list
member ConstructorArguments: (TType * objnull) list
member NamedArguments: (TType * string * bool * objnull) list
member Range: range
member TyconRef: TyconRef

Expand Down
12 changes: 6 additions & 6 deletions src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -3060,7 +3060,7 @@ module EstablishTypeDefinitionCores =

if not isRootGenerated then
let desig = theRootTypeWithRemapping.TypeProviderDesignation
let nm = theRootTypeWithRemapping.PUntaint((fun st -> st.FullName), m)
let nm = theRootTypeWithRemapping.PUntaint((fun st -> string st.FullName), m)
error(Error(FSComp.SR.etErasedTypeUsedInGeneration(desig, nm), m))

cenv.createsGeneratedProvidedTypes <- true
Expand Down Expand Up @@ -3101,7 +3101,7 @@ module EstablishTypeDefinitionCores =

if not isGenerated then
let desig = st.TypeProviderDesignation
let nm = st.PUntaint((fun st -> st.FullName), m)
let nm = st.PUntaint((fun st -> string st.FullName), m)
error(Error(FSComp.SR.etErasedTypeUsedInGeneration(desig, nm), m))

// Embed the type into the module we're compiling
Expand Down Expand Up @@ -4251,7 +4251,7 @@ module TcDeclarations =
// For historical reasons we only give a warning for incorrect type parameters on intrinsic extensions
if nReqTypars <> synTypars.Length then
errorR(Error(FSComp.SR.tcDeclaredTypeParametersForExtensionDoNotMatchOriginal(tcref.DisplayNameWithStaticParametersAndUnderscoreTypars), m))
if not (typarsAEquiv g TypeEquivEnv.Empty reqTypars declaredTypars) then
if not (typarsAEquiv g (TypeEquivEnv.EmptyWithNullChecks g) reqTypars declaredTypars) then
warning(Error(FSComp.SR.tcDeclaredTypeParametersForExtensionDoNotMatchOriginal(tcref.DisplayNameWithStaticParametersAndUnderscoreTypars), m))
// Note we return 'reqTypars' for intrinsic extensions since we may only have given warnings
IntrinsicExtensionBinding, reqTypars
Expand All @@ -4260,7 +4260,7 @@ module TcDeclarations =
errorR(Error(FSComp.SR.tcMembersThatExtendInterfaceMustBePlacedInSeparateModule(), tcref.Range))
if nReqTypars <> synTypars.Length then
error(Error(FSComp.SR.tcDeclaredTypeParametersForExtensionDoNotMatchOriginal(tcref.DisplayNameWithStaticParametersAndUnderscoreTypars), m))
if not (typarsAEquiv g TypeEquivEnv.Empty reqTypars declaredTypars) then
if not (typarsAEquiv g (TypeEquivEnv.EmptyWithNullChecks g) reqTypars declaredTypars) then
errorR(Error(FSComp.SR.tcDeclaredTypeParametersForExtensionDoNotMatchOriginal(tcref.DisplayNameWithStaticParametersAndUnderscoreTypars), m))
ExtrinsicExtensionBinding, declaredTypars

Expand Down Expand Up @@ -5702,7 +5702,7 @@ let CheckModuleSignature g (cenv: cenv) m denvAtEnd rootSigOpt implFileTypePrior
|]

// We want to show imperative type variables in any types in error messages at this late point
let denv = { denvAtEnd with showInferenceTyparAnnotations=true }
let denv = { denvAtEnd with showInferenceTyparAnnotations=true;showNullnessAnnotations=Some g.checkNullness }
try

// As typechecked the signature and implementation use different tycons etc.
Expand All @@ -5714,7 +5714,7 @@ let CheckModuleSignature g (cenv: cenv) m denvAtEnd rootSigOpt implFileTypePrior
// Compute the remapping from implementation to signature
let remapInfo, _ = ComputeRemappingFromInferredSignatureToExplicitSignature g implFileTypePriorToSig sigFileType

let aenv = { TypeEquivEnv.Empty with EquivTycons = TyconRefMap.OfList remapInfo.RepackagedEntities }
let aenv = { TypeEquivEnv.EmptyWithNullChecks g with EquivTycons = TyconRefMap.OfList remapInfo.RepackagedEntities }

if not (SignatureConformance.Checker(g, cenv.amap, denv, remapInfo, true).CheckSignature aenv cenv.infoReader (mkLocalModuleRef implFileSpecPriorToSig) sigFileType) then
// We can just raise 'ReportedError' since CheckModuleOrNamespace raises its own error
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/ConstraintSolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -345,7 +345,7 @@ let MakeConstraintSolverEnv contextInfo css m denv =
eContextInfo = contextInfo
MatchingOnly = false
ErrorOnFailedMemberConstraintResolution = false
EquivEnv = TypeEquivEnv.Empty
EquivEnv = TypeEquivEnv.EmptyIgnoreNulls
DisplayEnv = denv
IsSpeculativeForMethodOverloading = false
IsSupportsNullFlex = false
Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/Expressions/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -12687,7 +12687,7 @@ and FixupLetrecBind (cenv: cenv) denv generalizedTyparsForRecursiveBlock (bind:
| Some _ ->
match PartitionValTyparsForApparentEnclosingType g vspec with
| Some(parentTypars, memberParentTypars, _, _, _) ->
ignore(SignatureConformance.Checker(g, cenv.amap, denv, SignatureRepackageInfo.Empty, false).CheckTypars vspec.Range TypeEquivEnv.Empty memberParentTypars parentTypars)
ignore(SignatureConformance.Checker(g, cenv.amap, denv, SignatureRepackageInfo.Empty, false).CheckTypars vspec.Range TypeEquivEnv.EmptyIgnoreNulls memberParentTypars parentTypars)
| None ->
errorR(Error(FSComp.SR.tcMemberIsNotSufficientlyGeneric(), vspec.Range))
| _ -> ()
Expand Down
3 changes: 2 additions & 1 deletion src/Compiler/Checking/InfoReader.fs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,8 @@ let rec GetImmediateIntrinsicMethInfosOfTypeAux (optFilter, ad) g amap m withExp
let st = info.ProvidedType
let meths =
match optFilter with
| Some name -> st.PApplyArray ((fun st -> st.GetMethods() |> Array.filter (fun mi -> mi.Name = name) ), "GetMethods", m)
| Some name ->
st.PApplyFilteredArray ((fun st -> st.GetMethods()),(fun mi -> mi.Name = name), "GetMethods", m)
| None -> st.PApplyArray ((fun st -> st.GetMethods()), "GetMethods", m)
[ for mi in meths -> ProvidedMeth(amap, mi.Coerce(m), None, m) ]
#endif
Expand Down
14 changes: 7 additions & 7 deletions src/Compiler/Checking/MethodCalls.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1120,7 +1120,7 @@ let TryImportProvidedMethodBaseAsLibraryIntrinsic (amap: Import.ImportMap, m: ra
match tryTcrefOfAppTy amap.g declaringType with
| ValueSome declaringEntity ->
if not declaringEntity.IsLocalRef && ccuEq declaringEntity.nlr.Ccu amap.g.fslibCcu then
let n = mbase.PUntaint((fun x -> x.GetParameters().Length), m)
let n = mbase.PApplyArray((fun x -> x.GetParameters()),"GetParameters", m).Length
match amap.g.knownIntrinsics.TryGetValue ((declaringEntity.LogicalName, None, methodName, n)) with
| true, vref -> Some vref
| _ ->
Expand Down Expand Up @@ -1815,14 +1815,14 @@ module ProvidedMethodCalls =
let rec loop (st: Tainted<ProvidedType>) =
if st.PUntaint((fun st -> st.IsGenericParameter), m) then st
elif st.PUntaint((fun st -> st.IsArray), m) then
let et = st.PApply((fun st -> st.GetElementType()), m)
let et = st.PApply((fun st -> !! st.GetElementType()), m)
let rank = st.PUntaint((fun st -> st.GetArrayRank()), m)
(loop et).PApply((fun st -> if rank = 1 then st.MakeArrayType() else st.MakeArrayType(rank)), m)
elif st.PUntaint((fun st -> st.IsByRef), m) then
let et = st.PApply((fun st -> st.GetElementType()), m)
let et = st.PApply((fun st -> !! st.GetElementType()), m)
(loop et).PApply((fun st -> st.MakeByRefType()), m)
elif st.PUntaint((fun st -> st.IsPointer), m) then
let et = st.PApply((fun st -> st.GetElementType()), m)
let et = st.PApply((fun st -> !! st.GetElementType()), m)
(loop et).PApply((fun st -> st.MakePointerType()), m)
else
let isGeneric = st.PUntaint((fun st -> st.IsGenericType), m)
Expand Down Expand Up @@ -1863,7 +1863,7 @@ module ProvidedMethodCalls =
allArgs: Exprs,
paramVars: Tainted<ProvidedVar>[],
g, amap, mut, isProp, isSuperInit, m,
expr: Tainted<ProvidedExpr>) =
expr: Tainted<ProvidedExpr MaybeNull>) =

let varConv =
// note: Assuming the size based on paramVars
Expand All @@ -1873,7 +1873,7 @@ module ProvidedMethodCalls =
dict.Add(v, (None, e))
dict

let rec exprToExprAndWitness top (ea: Tainted<ProvidedExpr>) =
let rec exprToExprAndWitness top (ea: Tainted<ProvidedExpr MaybeNull>) =
let fail() = error(Error(FSComp.SR.etUnsupportedProvidedExpression(ea.PUntaint((fun etree -> etree.UnderlyingExpressionString), m)), m))
match ea with
| Tainted.Null -> error(Error(FSComp.SR.etNullProvidedExpression(ea.TypeProviderDesignation), m))
Expand Down Expand Up @@ -2115,7 +2115,7 @@ module ProvidedMethodCalls =
methInfoOpt, expr, exprTy
with
| :? TypeProviderError as tpe ->
let typeName = mi.PUntaint((fun mb -> (nonNull<ProvidedType> mb.DeclaringType).FullName), m)
let typeName = mi.PUntaint((fun mb -> (nonNull<ProvidedType> mb.DeclaringType).FullName |> string), m)
let methName = mi.PUntaint((fun mb -> mb.Name), m)
raise( tpe.WithContext(typeName, methName) ) // loses original stack trace
#endif
Expand Down
4 changes: 2 additions & 2 deletions src/Compiler/Checking/MethodOverrides.fs
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ module DispatchSlotChecking =
// Compare the types. CompiledSigOfMeth, GetObjectExprOverrideInfo and GetTypeMemberOverrideInfo have already
// applied all relevant substitutions except the renamings from fvtmps <-> methTypars

let aenv = TypeEquivEnv.FromEquivTypars fvmethTypars methTypars
let aenv = (TypeEquivEnv.EmptyIgnoreNulls).FromEquivTypars fvmethTypars methTypars

List.forall2 (List.lengthsEqAndForall2 (typeAEquiv g aenv)) vargTys argTys &&
returnTypesAEquiv g aenv vrty retTy &&
Expand Down Expand Up @@ -305,7 +305,7 @@ module DispatchSlotChecking =
ComposeTyparInsts ttpinst (ReverseTyparRenaming g memberToParentInst)

// Compare under the composed substitutions
let aenv = TypeEquivEnv.FromTyparInst ttpinst
let aenv = (TypeEquivEnv.EmptyIgnoreNulls).FromTyparInst ttpinst

typarsAEquiv g aenv fvmethTypars methTypars

Expand Down
2 changes: 1 addition & 1 deletion src/Compiler/Checking/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ let ResolveProvidedTypeNameInEntity (amap, m, typeName, modref: ModuleOrNamespac
//if staticResInfo.NumStaticArgs > 0 then
// error(Error(FSComp.SR.etNestedProvidedTypesDoNotTakeStaticArgumentsOrGenericParameters(), m))
[]
| nestedSty ->
| Tainted.NonNull nestedSty ->
[AddEntityForProvidedType (amap, modref, resolutionEnvironment, nestedSty, m) ]
| _ -> []
#endif
Expand Down
6 changes: 3 additions & 3 deletions src/Compiler/Checking/NicePrint.fs
Original file line number Diff line number Diff line change
Expand Up @@ -764,7 +764,7 @@ module PrintTypes =
|> ListSet.setify (fun (_, cx1) (_, cx2) ->
match cx1, cx2 with
| TyparConstraint.MayResolveMember(traitInfo1, _),
TyparConstraint.MayResolveMember(traitInfo2, _) -> traitsAEquiv denv.g TypeEquivEnv.Empty traitInfo1 traitInfo2
TyparConstraint.MayResolveMember(traitInfo2, _) -> traitsAEquiv denv.g (TypeEquivEnv.EmptyWithNullChecks denv.g) traitInfo1 traitInfo2
| _ -> false)

let cxsL = List.collect (layoutConstraintWithInfo denv env) cxs
Expand Down Expand Up @@ -2177,7 +2177,7 @@ module TastDefinitionPrinting =
match tcref.TypeReprInfo with
| TProvidedTypeRepr info ->
[
for nestedType in info.ProvidedType.PApplyArray((fun sty -> sty.GetNestedTypes() |> Array.filter (fun t -> t.IsPublic || t.IsNestedPublic)), "GetNestedTypes", m) do
for nestedType in info.ProvidedType.PApplyFilteredArray((fun sty -> sty.GetNestedTypes()),(fun t -> t.IsPublic || t.IsNestedPublic), "GetNestedTypes", m) do
yield nestedType.PUntaint((fun t -> t.IsClass, t.Name), m)
]
|> List.sortBy snd
Expand Down Expand Up @@ -2930,7 +2930,7 @@ let minimalStringsOfTwoTypes denv ty1 ty2 =
let denv = denv.SetOpenPaths []
let denv = { denv with includeStaticParametersInTypeNames=true }
let makeName t =
let assemblyName = PrintTypes.layoutAssemblyName denv t |> function Null | NonNull "" -> "" | NonNull name -> sprintf " (%s)" name
let assemblyName = PrintTypes.layoutAssemblyName denv t |> function | "" -> "" | name -> $" (%s{name})"
sprintf "%s%s" (stringOfTy denv t) assemblyName

(makeName ty1, makeName ty2, stringOfTyparConstraints denv tpcs)
Expand Down
Loading

0 comments on commit 841ba8e

Please sign in to comment.