Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Name resolution: keep type vars in subsequent checks #16456

Merged
merged 15 commits into from
Jan 16, 2024
3 changes: 2 additions & 1 deletion docs/release-notes/.FSharp.Compiler.Service/8.0.200.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
* Parser recovers on unfinished enum case declarations. ([PR #16401](https://github.com/dotnet/fsharp/pull/16401))
* Parser recovers on unfinished record declarations. ([PR #16357](https://github.com/dotnet/fsharp/pull/16357))
* `MutableKeyword` to [SynFieldTrivia](../reference/fsharp-compiler-syntaxtrivia-synfieldtrivia.html) ([PR #16357](https://github.com/dotnet/fsharp/pull/16357))
* Name resolution: keep type vars in subsequent checks ([PR #16456](https://github.com/dotnet/fsharp/pull/16456))
auduchinok marked this conversation as resolved.
Show resolved Hide resolved
* Added support for a new parameterless constructor for `CustomOperationAttribute`, which, when applied, will use method name as keyword for custom operation in computation expression builder. ([PR #16475](https://github.com/dotnet/fsharp/pull/16475), part of implementation for [fslang-suggestions/1250](https://github.com/fsharp/fslang-suggestions/issues/1250))

### Changed

* Speed up unused opens handling for empty results. ([PR #16502](https://github.com/dotnet/fsharp/pull/16502))
* Speed up unused opens handling for empty results. ([PR #16502](https://github.com/dotnet/fsharp/pull/16502))
17 changes: 9 additions & 8 deletions src/Compiler/Checking/CheckDeclarations.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2941,7 +2941,7 @@ module EstablishTypeDefinitionCores =
| Some (tc, args, m) ->
let ad = envinner.AccessRights
match ResolveTypeLongIdent cenv.tcSink cenv.nameResolver ItemOccurence.UseInType OpenQualified envinner.NameEnv ad tc TypeNameResolutionStaticArgsInfo.DefiniteEmpty PermitDirectReferenceToGeneratedType.Yes with
| Result (_, tcrefBeforeStaticArguments) when
| Result (_, tcrefBeforeStaticArguments, _) when
tcrefBeforeStaticArguments.IsProvided &&
not tcrefBeforeStaticArguments.IsErased ->

Expand Down Expand Up @@ -4117,11 +4117,11 @@ module TcDeclarations =

| _ ->
let resInfo = TypeNameResolutionStaticArgsInfo.FromTyArgs synTypars.Length
let _, tcref =
let tcref =
match ResolveTypeLongIdent cenv.tcSink cenv.nameResolver ItemOccurence.Binding OpenQualified envForDecls.NameEnv ad longPath resInfo PermitDirectReferenceToGeneratedType.No with
| Result res ->
// Update resolved type parameters with the names from the source.
let _, tcref = res
let _, tcref, _ = res
if tcref.TyparsNoRange.Length = synTypars.Length then
(tcref.TyparsNoRange, synTypars)
||> List.zip
Expand All @@ -4131,11 +4131,12 @@ module TcDeclarations =
typar.SetIdent(untypedIdent)
)

res
| res when inSig && List.isSingleton longPath ->
errorR(Deprecated(FSComp.SR.tcReservedSyntaxForAugmentation(), m))
ForceRaise res
| res -> ForceRaise res
tcref

| Exception exn ->
if inSig && List.isSingleton longPath then
errorR(Deprecated(FSComp.SR.tcReservedSyntaxForAugmentation(), m))
ForceRaise (Exception exn)
tcref

let isInterfaceOrDelegateOrEnum =
Expand Down
42 changes: 24 additions & 18 deletions src/Compiler/Checking/CheckExpressions.fs
Original file line number Diff line number Diff line change
Expand Up @@ -767,7 +767,7 @@ let TcConst (cenv: cenv) (overallTy: TType) m env synConst =
| SynMeasure.One _ -> Measure.One
| SynMeasure.Named(tc, m) ->
let ad = env.eAccessRights
let _, tcref = ForceRaise(ResolveTypeLongIdent cenv.tcSink cenv.nameResolver ItemOccurence.Use OpenQualified env.eNameResEnv ad tc TypeNameResolutionStaticArgsInfo.DefiniteEmpty PermitDirectReferenceToGeneratedType.No)
let _, tcref, _ = ForceRaise(ResolveTypeLongIdent cenv.tcSink cenv.nameResolver ItemOccurence.Use OpenQualified env.eNameResEnv ad tc TypeNameResolutionStaticArgsInfo.DefiniteEmpty PermitDirectReferenceToGeneratedType.No)
match tcref.TypeOrMeasureKind with
| TyparKind.Type -> error(Error(FSComp.SR.tcExpectedUnitOfMeasureNotType(), m))
| TyparKind.Measure -> Measure.Const tcref
Expand Down Expand Up @@ -4458,7 +4458,7 @@ and TcLongIdentType kindOpt (cenv: cenv) newOk checkConstraints occ iwsam env tp
let m = synLongId.Range
let ad = env.eAccessRights

let tinstEnclosing, tcref = ForceRaise(ResolveTypeLongIdent cenv.tcSink cenv.nameResolver occ OpenQualified env.NameEnv ad tc TypeNameResolutionStaticArgsInfo.DefiniteEmpty PermitDirectReferenceToGeneratedType.No)
let tinstEnclosing, tcref, inst = ForceRaise(ResolveTypeLongIdent cenv.tcSink cenv.nameResolver occ OpenQualified env.NameEnv ad tc TypeNameResolutionStaticArgsInfo.DefiniteEmpty PermitDirectReferenceToGeneratedType.No)

CheckIWSAM cenv env checkConstraints iwsam m tcref

Expand All @@ -4472,15 +4472,15 @@ and TcLongIdentType kindOpt (cenv: cenv) newOk checkConstraints occ iwsam env tp
| _, TyparKind.Measure ->
TType_measure (Measure.Const tcref), tpenv
| _, TyparKind.Type ->
TcTypeApp cenv newOk checkConstraints occ env tpenv m tcref tinstEnclosing []
TcTypeApp cenv newOk checkConstraints occ env tpenv m tcref tinstEnclosing [] inst

/// Some.Long.TypeName<ty1, ty2>
/// ty1 SomeLongTypeName
and TcLongIdentAppType kindOpt (cenv: cenv) newOk checkConstraints occ iwsam env tpenv longId postfix args m =
let (SynLongIdent(tc, _, _)) = longId
let ad = env.eAccessRights

let tinstEnclosing, tcref =
let tinstEnclosing, tcref, inst =
let tyResInfo = TypeNameResolutionStaticArgsInfo.FromTyArgs args.Length
ResolveTypeLongIdent cenv.tcSink cenv.nameResolver ItemOccurence.UseInType OpenQualified env.eNameResEnv ad tc tyResInfo PermitDirectReferenceToGeneratedType.No
|> ForceRaise
Expand All @@ -4499,7 +4499,7 @@ and TcLongIdentAppType kindOpt (cenv: cenv) newOk checkConstraints occ iwsam env
| _, TyparKind.Type ->
if postfix && tcref.Typars m |> List.exists (fun tp -> match tp.Kind with TyparKind.Measure -> true | _ -> false) then
error(Error(FSComp.SR.tcInvalidUnitsOfMeasurePrefix(), m))
TcTypeApp cenv newOk checkConstraints occ env tpenv m tcref tinstEnclosing args
TcTypeApp cenv newOk checkConstraints occ env tpenv m tcref tinstEnclosing args inst

| _, TyparKind.Measure ->
match args, postfix with
Expand All @@ -4518,8 +4518,8 @@ and TcNestedAppType (cenv: cenv) newOk checkConstraints occ iwsam env tpenv synL
let leftTy, tpenv = TcType cenv newOk checkConstraints occ iwsam env tpenv synLeftTy
match leftTy with
| AppTy g (tcref, tinst) ->
let tcref = ResolveTypeLongIdentInTyconRef cenv.tcSink cenv.nameResolver env.eNameResEnv (TypeNameResolutionInfo.ResolveToTypeRefs (TypeNameResolutionStaticArgsInfo.FromTyArgs args.Length)) ad m tcref longId
TcTypeApp cenv newOk checkConstraints occ env tpenv m tcref tinst args
let tcref, inst = ResolveTypeLongIdentInTyconRef cenv.tcSink cenv.nameResolver env.eNameResEnv (TypeNameResolutionInfo.ResolveToTypeRefs (TypeNameResolutionStaticArgsInfo.FromTyArgs args.Length)) ad m tcref longId
TcTypeApp cenv newOk checkConstraints occ env tpenv m tcref tinst args inst
| _ ->
error(Error(FSComp.SR.tcTypeHasNoNestedTypes(), m))

Expand Down Expand Up @@ -4943,7 +4943,7 @@ and TcProvidedTypeApp (cenv: cenv) env tpenv tcref args m =
/// Note that the generic type may be a nested generic type List<T>.ListEnumerator<U>.
/// In this case, 'argsR is only the instantiation of the suffix type arguments, and pathTypeArgs gives
/// the prefix of type arguments.
and TcTypeApp (cenv: cenv) newOk checkConstraints occ env tpenv m tcref pathTypeArgs (synArgTys: SynType list) =
and TcTypeApp (cenv: cenv) newOk checkConstraints occ env tpenv m tcref pathTypeArgs (synArgTys: SynType list) (tinst: TypeInst) =
let g = cenv.g
CheckTyconAccessible cenv.amap m env.AccessRights tcref |> ignore
CheckEntityAttributes g tcref m |> CommitOperationResult
Expand All @@ -4954,16 +4954,22 @@ and TcTypeApp (cenv: cenv) newOk checkConstraints occ env tpenv m tcref pathType
if tcref.Deref.IsProvided then TcProvidedTypeApp cenv env tpenv tcref synArgTys m else
#endif

let tps, _, tinst, _ = FreshenTyconRef2 g m tcref

// If we're not checking constraints, i.e. when we first assert the super/interfaces of a type definition, then just
// clear the constraint lists of the freshly generated type variables. A little ugly but fairly localized.
if checkConstraints = NoCheckCxs then tps |> List.iter (fun tp -> tp.SetConstraints [])
let synArgTysLength = synArgTys.Length
let pathTypeArgsLength = pathTypeArgs.Length
if tinst.Length <> pathTypeArgsLength + synArgTysLength then
error (TyconBadArgs(env.DisplayEnv, tcref, pathTypeArgsLength + synArgTysLength, m))

let tps = tinst |> List.skip pathTypeArgsLength |> List.map (fun t ->
match t with
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't probably match on ttype directly, need do stripping first

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure of this holds for a type instantiation, but it is a good point to add a test case with a type alias as a typar to see what it does.

If we stripped, we would lose the type alias and then it couldn't be searched for as a separate symbol... ?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure it'd be what we want to achieve here. I'm unwrapping the type variables produced by name resolution logic here, I probably don't want to change them in any way. Another approach could be to refactor more places and to pass the typar instantiation explicitly, but it seems that simply unwrapping it like this works for the cases we're interested in here, i.e. to make the types flow into subsequent checking phases after reporting.

| TType_var(typar, _)
| TType_measure(Measure.Var typar) -> typar
| t -> failwith $"TcTypeApp: {t}"
)

// If we're not checking constraints, i.e. when we first assert the super/interfaces of a type definition, then just
// clear the constraint lists of the freshly generated type variables. A little ugly but fairly localized.
if checkConstraints = NoCheckCxs then tps |> List.iter (fun tp -> tp.SetConstraints [])

let argTys, tpenv =
// Get the suffix of typars
let tpsForArgs = List.skip (tps.Length - synArgTysLength) tps
Expand Down Expand Up @@ -5009,9 +5015,9 @@ and TcNestedTypeApplication (cenv: cenv) newOk checkConstraints occ iwsam env tp
error(Error(FSComp.SR.tcTypeHasNoNestedTypes(), mWholeTypeApp))

match ty with
| TType_app(tcref, _, _) ->
| TType_app(tcref, inst, _) ->
CheckIWSAM cenv env checkConstraints iwsam mWholeTypeApp tcref
TcTypeApp cenv newOk checkConstraints occ env tpenv mWholeTypeApp tcref pathTypeArgs tyargs
TcTypeApp cenv newOk checkConstraints occ env tpenv mWholeTypeApp tcref pathTypeArgs tyargs inst
| _ ->
error(InternalError("TcNestedTypeApplication: expected type application", mWholeTypeApp))

Expand Down Expand Up @@ -8163,10 +8169,10 @@ and TcNameOfExpr (cenv: cenv) env tpenv (synArg: SynExpr) =
if (match delayed with [DelayedTypeApp _] | [] -> true | _ -> false) then
let (TypeNameResolutionInfo(_, staticArgsInfo)) = GetLongIdentTypeNameInfo delayed
match ResolveTypeLongIdent cenv.tcSink cenv.nameResolver ItemOccurence.UseInAttribute OpenQualified env.eNameResEnv ad longId staticArgsInfo PermitDirectReferenceToGeneratedType.No with
| Result (tinstEnclosing, tcref) when IsEntityAccessible cenv.amap m ad tcref ->
| Result (tinstEnclosing, tcref, inst) when IsEntityAccessible cenv.amap m ad tcref ->
match delayed with
| [DelayedTypeApp (tyargs, _, mExprAndTypeArgs)] ->
TcTypeApp cenv NewTyparsOK CheckCxs ItemOccurence.UseInType env tpenv mExprAndTypeArgs tcref tinstEnclosing tyargs |> ignore
TcTypeApp cenv NewTyparsOK CheckCxs ItemOccurence.UseInType env tpenv mExprAndTypeArgs tcref tinstEnclosing tyargs inst |> ignore
| _ -> ()
true // resolved to a type name, done with checks
| _ ->
Expand Down Expand Up @@ -10858,7 +10864,7 @@ and TcAttributeEx canFail (cenv: cenv) (env: TcEnv) attrTgt attrEx (synAttr: Syn

match ResolveTypeLongIdent cenv.tcSink cenv.nameResolver ItemOccurence.UseInAttribute OpenQualified env.eNameResEnv ad tycon TypeNameResolutionStaticArgsInfo.DefiniteEmpty PermitDirectReferenceToGeneratedType.No with
| Exception err -> raze err
| Result(tinstEnclosing, tcref) -> success(TcTypeApp cenv NoNewTypars CheckCxs ItemOccurence.UseInAttribute env tpenv mAttr tcref tinstEnclosing [])
| Result(tinstEnclosing, tcref, inst) -> success(TcTypeApp cenv NoNewTypars CheckCxs ItemOccurence.UseInAttribute env tpenv mAttr tcref tinstEnclosing [] inst)

ForceRaise ((try1 (tyid.idText + "Attribute")) |> otherwise (fun () -> (try1 tyid.idText)))

Expand Down
85 changes: 3 additions & 82 deletions src/Compiler/Checking/ConstraintSolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -57,97 +57,18 @@ open FSharp.Compiler.Import
open FSharp.Compiler.InfoReader
open FSharp.Compiler.Infos
open FSharp.Compiler.MethodCalls
open FSharp.Compiler.NameResolution
open FSharp.Compiler.Syntax
open FSharp.Compiler.Syntax.PrettyNaming
open FSharp.Compiler.SyntaxTreeOps
open FSharp.Compiler.TcGlobals
open FSharp.Compiler.Text
open FSharp.Compiler.Text.Range
open FSharp.Compiler.TypedTree
open FSharp.Compiler.TypedTreeBasics
open FSharp.Compiler.TypedTreeOps
open FSharp.Compiler.TypeHierarchy
open FSharp.Compiler.TypeRelations

//-------------------------------------------------------------------------
// Generate type variables and record them in within the scope of the
// compilation environment, which currently corresponds to the scope
// of the constraint resolution carried out by type checking.
//-------------------------------------------------------------------------

let compgenId = mkSynId range0 unassignedTyparName

let NewCompGenTypar (kind, rigid, staticReq, dynamicReq, error) =
Construct.NewTypar(kind, rigid, SynTypar(compgenId, staticReq, true), error, dynamicReq, [], false, false)

let AnonTyparId m = mkSynId m unassignedTyparName

let NewAnonTypar (kind, m, rigid, var, dyn) =
Construct.NewTypar (kind, rigid, SynTypar(AnonTyparId m, var, true), false, dyn, [], false, false)

let NewNamedInferenceMeasureVar (_m, rigid, var, id) =
Construct.NewTypar(TyparKind.Measure, rigid, SynTypar(id, var, false), false, TyparDynamicReq.No, [], false, false)

let NewInferenceMeasurePar () =
NewCompGenTypar (TyparKind.Measure, TyparRigidity.Flexible, TyparStaticReq.None, TyparDynamicReq.No, false)

let NewErrorTypar () =
NewCompGenTypar (TyparKind.Type, TyparRigidity.Flexible, TyparStaticReq.None, TyparDynamicReq.No, true)

let NewErrorMeasureVar () =
NewCompGenTypar (TyparKind.Measure, TyparRigidity.Flexible, TyparStaticReq.None, TyparDynamicReq.No, true)

let NewInferenceType (g: TcGlobals) =
ignore g // included for future, minimizing code diffs, see https://github.com/dotnet/fsharp/pull/6804
mkTyparTy (Construct.NewTypar (TyparKind.Type, TyparRigidity.Flexible, SynTypar(compgenId, TyparStaticReq.None, true), false, TyparDynamicReq.No, [], false, false))

let NewErrorType () =
mkTyparTy (NewErrorTypar ())

let NewErrorMeasure () =
Measure.Var (NewErrorMeasureVar ())

let NewByRefKindInferenceType (g: TcGlobals) m =
let tp = Construct.NewTypar (TyparKind.Type, TyparRigidity.Flexible, SynTypar(compgenId, TyparStaticReq.HeadType, true), false, TyparDynamicReq.No, [], false, false)
if g.byrefkind_InOut_tcr.CanDeref then
tp.SetConstraints [TyparConstraint.DefaultsTo(10, TType_app(g.byrefkind_InOut_tcr, [], g.knownWithoutNull), m)]
mkTyparTy tp

let NewInferenceTypes g l = l |> List.map (fun _ -> NewInferenceType g)

let FreshenTypar (g: TcGlobals) rigid (tp: Typar) =
let clearStaticReq = g.langVersion.SupportsFeature LanguageFeature.InterfacesWithAbstractStaticMembers
let staticReq = if clearStaticReq then TyparStaticReq.None else tp.StaticReq
let dynamicReq = if rigid = TyparRigidity.Rigid then TyparDynamicReq.Yes else TyparDynamicReq.No
NewCompGenTypar (tp.Kind, rigid, staticReq, dynamicReq, false)

// QUERY: should 'rigid' ever really be 'true'? We set this when we know
// we are going to have to generalize a typar, e.g. when implementing a
// abstract generic method slot. But we later check the generalization
// condition anyway, so we could get away with a non-rigid typar. This
// would sort of be cleaner, though give errors later.
let FreshenAndFixupTypars g m rigid fctps tinst tpsorig =
let tps = tpsorig |> List.map (FreshenTypar g rigid)
let renaming, tinst = FixupNewTypars m fctps tinst tpsorig tps
tps, renaming, tinst

let FreshenTypeInst g m tpsorig =
FreshenAndFixupTypars g m TyparRigidity.Flexible [] [] tpsorig

let FreshMethInst g m fctps tinst tpsorig =
FreshenAndFixupTypars g m TyparRigidity.Flexible fctps tinst tpsorig

let FreshenTypars g m tpsorig =
match tpsorig with
| [] -> []
| _ ->
let _, _, tpTys = FreshenTypeInst g m tpsorig
tpTys

let FreshenMethInfo m (minfo: MethInfo) =
let _, _, tpTys = FreshMethInst minfo.TcGlobals m (minfo.GetFormalTyparsOfDeclaringType m) minfo.DeclaringTypeInst minfo.FormalMethodTypars
tpTys

//-------------------------------------------------------------------------
// Unification of types: solve/record equality constraints
// Subsumption of types: solve/record subtyping constraints
Expand Down Expand Up @@ -1718,8 +1639,8 @@ and SolveMemberConstraint (csenv: ConstraintSolverEnv) ignoreUnresolvedOverload
let propName = nm[4..]
let props =
supportTys |> List.choose (fun ty ->
match NameResolution.TryFindAnonRecdFieldOfType g ty propName with
| Some (NameResolution.Item.AnonRecdField(anonInfo, tinst, i, _)) -> Some (anonInfo, tinst, i)
match TryFindAnonRecdFieldOfType g ty propName with
| Some (Item.AnonRecdField(anonInfo, tinst, i, _)) -> Some (anonInfo, tinst, i)
| _ -> None)
match props with
| [ prop ] -> Some prop
Expand Down
55 changes: 0 additions & 55 deletions src/Compiler/Checking/ConstraintSolver.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -15,61 +15,6 @@ open FSharp.Compiler.Text
open FSharp.Compiler.TypedTree
open FSharp.Compiler.TypedTreeOps

/// Create a type variable representing the use of a "_" in F# code
val NewAnonTypar: TyparKind * range * TyparRigidity * TyparStaticReq * TyparDynamicReq -> Typar

/// Create an inference type variable
val NewInferenceType: TcGlobals -> TType

/// Create an inference type variable for the kind of a byref pointer
val NewByRefKindInferenceType: TcGlobals -> range -> TType

/// Create an inference type variable representing an error condition when checking an expression
val NewErrorType: unit -> TType

/// Create an inference type variable representing an error condition when checking a measure
val NewErrorMeasure: unit -> Measure

/// Create a list of inference type variables, one for each element in the input list
val NewInferenceTypes: TcGlobals -> 'T list -> TType list

/// Given a set of formal type parameters and their constraints, make new inference type variables for
/// each and ensure that the constraints on the new type variables are adjusted to refer to these.
///
/// Returns
/// 1. the new type parameters
/// 2. the instantiation mapping old type parameters to inference variables
/// 3. the inference type variables as a list of types.
val FreshenAndFixupTypars:
g: TcGlobals ->
m: range ->
rigid: TyparRigidity ->
Typars ->
TType list ->
Typars ->
Typars * TyparInstantiation * TType list

/// Given a set of type parameters, make new inference type variables for
/// each and ensure that the constraints on the new type variables are adjusted.
///
/// Returns
/// 1. the new type parameters
/// 2. the instantiation mapping old type parameters to inference variables
/// 3. the inference type variables as a list of types.
val FreshenTypeInst: g: TcGlobals -> range -> Typars -> Typars * TyparInstantiation * TType list

/// Given a set of type parameters, make new inference type variables for
/// each and ensure that the constraints on the new type variables are adjusted.
///
/// Returns the inference type variables as a list of types.
val FreshenTypars: g: TcGlobals -> range -> Typars -> TType list

/// Given a method, which may be generic, make new inference type variables for
/// its generic parameters, and ensure that the constraints the new type variables are adjusted.
///
/// Returns the inference type variables as a list of types.
val FreshenMethInfo: range -> MethInfo -> TType list

/// Information about the context of a type equation.
[<RequireQualifiedAccess>]
type ContextInfo =
Expand Down
Loading
Loading