Skip to content

Commit

Permalink
skip unloadable interfaces
Browse files Browse the repository at this point in the history
When the assembly reference set for the compilation is incomplete, some
nterfaces to types relevant to compilation may lie in assemblies outside
the assembly reference set. This applies particularly to private ("internals
visible to") interfaces found in .NET assemblies.

This causes very substantial usability bugs in practice as various parts of
type inference and other checking "give up" when you get this condition. The
C# compiler doesn't give up in the same way.

In most cases it is reasonable to simply skip interfaces-that-lie-outside-the-
set-of-referenced-assemblies during F# compilation. Skipping unresolvable
interfaces is pretty much harmless: any substantive analysis on the interface
type (such as implementing it) will require the assembly holding the interface
type.

There are some exceptions: if an interface I1 lies outside the referenceable
set and you try to implement I2 inheriting from I1 then we'd better not skip
I1. Indeed if you even try to find the methods on I2 then we'd better not skip
I1. These are covered by "FoldPrimaryHierarchyOfType" in the code.

fixes #337
closes #356

commit dd5205c769828e2e16e736c126cb62d68e7beb87
Author: Don Syme <[email protected]>
Date:   Thu Apr 23 23:53:59 2015 +0100

    add test case

commit db28771c75d022247dee6dea60a89b8c29fab4b5
Author: Don Syme <[email protected]>
Date:   Fri Apr 10 12:18:50 2015 +0200

    skip unloadable interfaces (2)

commit 18a47124480efdf58a75ce2fa7273c6f7550c1c0
Author: Don Syme <[email protected]>
Date:   Fri Apr 10 11:53:39 2015 +0200

    skip unloadable interfaces
  • Loading branch information
dsyme authored and latkin committed Apr 24, 2015
1 parent 90777db commit d52230b
Show file tree
Hide file tree
Showing 8 changed files with 86 additions and 21 deletions.
2 changes: 1 addition & 1 deletion src/fsharp/NicePrint.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1419,7 +1419,7 @@ module private TastDefinitionPrinting =
if suppressInheritanceAndInterfacesForTyInSimplifiedDisplays g amap m ty then
[]
else
GetImmediateInterfacesOfType g amap m ty |> List.map (fun ity -> wordL (if isInterfaceTy g ty then "inherit" else "interface") --- layoutType denv ity)
GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m ty |> List.map (fun ity -> wordL (if isInterfaceTy g ty then "inherit" else "interface") --- layoutType denv ity)

let props =
GetIntrinsicPropInfosOfType infoReader (None,ad,AllowMultiIntfInstantiations.Yes) PreferOverrides m ty
Expand Down
2 changes: 1 addition & 1 deletion src/fsharp/check.fs
Original file line number Diff line number Diff line change
Expand Up @@ -1369,7 +1369,7 @@ let CheckEntityDefn cenv env (tycon:Entity) =
if cenv.reportErrors then
if not tycon.IsTypeAbbrev then
let typ = generalizedTyconRef (mkLocalTyconRef tycon)
let immediateInterfaces = GetImmediateInterfacesOfType cenv.g cenv.amap m typ
let immediateInterfaces = GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes cenv.g cenv.amap m typ
let interfaces =
[ for ty in immediateInterfaces do
yield! AllSuperTypesOfType cenv.g cenv.amap m AllowMultiIntfInstantiations.Yes ty ]
Expand Down
27 changes: 27 additions & 0 deletions src/fsharp/import.fs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,16 @@ type ImportMap(g:TcGlobals,assemblyLoader:AssemblyLoader) =
member this.assemblyLoader = assemblyLoader
member this.ILTypeRefToTyconRefCache = typeRefToTyconRefCache

let CanImportILScopeRef (env:ImportMap) m scoref =
match scoref with
| ILScopeRef.Local -> true
| ILScopeRef.Module _ -> true
| ILScopeRef.Assembly assref ->
match env.assemblyLoader.LoadAssembly (m,assref) with
| UnresolvedCcu _ -> false
| ResolvedCcu _ -> true


/// Import a reference to a type definition, given the AbstractIL data for the type reference
let ImportTypeRefData (env:ImportMap) m (scoref,path,typeName) =
let ccu =
Expand Down Expand Up @@ -123,6 +133,10 @@ let ImportILTypeRef (env:ImportMap) m (tref:ILTypeRef) =
env.ILTypeRefToTyconRefCache.[tref] <- tcref
tcref

/// Import a reference to a type definition, given an AbstractIL ILTypeRef, with caching
let CanImportILTypeRef (env:ImportMap) m (tref:ILTypeRef) =
env.ILTypeRefToTyconRefCache.ContainsKey(tref) || CanImportILScopeRef env m tref.Scope

/// Import a type, given an AbstractIL ILTypeRef and an F# type instantiation.
///
/// Prefer the F# abbreviation for some built-in types, e.g. 'string' rather than
Expand Down Expand Up @@ -159,6 +173,19 @@ let rec ImportILType (env:ImportMap) m tinst typ =
with _ ->
error(Error(FSComp.SR.impNotEnoughTypeParamsInScopeWhileImporting(),m))

let rec CanImportILType (env:ImportMap) m typ =
match typ with
| ILType.Void -> true
| ILType.Array(_bounds,ty) -> CanImportILType env m ty
| ILType.Boxed tspec | ILType.Value tspec ->
CanImportILTypeRef env m tspec.TypeRef
&& tspec.GenericArgs |> ILList.toList |> List.forall (CanImportILType env m)
| ILType.Byref ty -> CanImportILType env m ty
| ILType.Ptr ty -> CanImportILType env m ty
| ILType.FunctionPointer _ -> true
| ILType.Modified(_,_,ty) -> CanImportILType env m ty
| ILType.TypeVar _u16 -> true

#if EXTENSIONTYPING

/// Import a provided type reference as an F# type TyconRef
Expand Down
6 changes: 6 additions & 0 deletions src/fsharp/import.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,15 @@ type ImportMap =
/// Import a reference to a type definition, given an AbstractIL ILTypeRef, with caching
val internal ImportILTypeRef : ImportMap -> range -> ILTypeRef -> TyconRef

/// Pre-check for ability to import a reference to a type definition, given an AbstractIL ILTypeRef, with caching
val internal CanImportILTypeRef : ImportMap -> range -> ILTypeRef -> bool

/// Import an IL type as an F# type.
val internal ImportILType : ImportMap -> range -> TType list -> ILType -> TType

/// Pre-check for ability to import an IL type as an F# type.
val internal CanImportILType : ImportMap -> range -> ILType -> bool

#if EXTENSIONTYPING

/// Import a provided type as an F# type.
Expand Down
51 changes: 36 additions & 15 deletions src/fsharp/infos.fs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,9 @@ open Microsoft.FSharp.Core.CompilerServices
let ImportType scoref amap m importInst ilty =
ilty |> rescopeILType scoref |> Import.ImportILType amap m importInst

let CanImportType scoref amap m ilty =
ilty |> rescopeILType scoref |> Import.CanImportILType amap m

//-------------------------------------------------------------------------
// Fold the hierarchy.
// REVIEW: this code generalizes the iteration used below for member lookup.
Expand Down Expand Up @@ -86,17 +89,21 @@ let GetSuperTypeOfType g amap m typ =
/// Make a type for System.Collections.Generic.IList<ty>
let mkSystemCollectionsGenericIListTy g ty = TType_app(g.tcref_System_Collections_Generic_IList,[ty])

[<RequireQualifiedAccess>]
/// Indicates whether we can skip interface types that lie outside the reference set
type SkipUnrefInterfaces = Yes | No


/// Collect the set of immediate declared interface types for an F# type, but do not
/// traverse the type hierarchy to collect further interfaces.
let rec GetImmediateInterfacesOfType g amap m typ =
let rec GetImmediateInterfacesOfType skipUnref g amap m typ =
let itys =
if isAppTy g typ then
let tcref,tinst = destAppTy g typ
if tcref.IsMeasureableReprTycon then
[ match tcref.TypeReprInfo with
| TMeasureableRepr reprTy ->
for ity in GetImmediateInterfacesOfType g amap m reprTy do
for ity in GetImmediateInterfacesOfType skipUnref g amap m reprTy do
if isAppTy g ity then
let itcref = tcrefOfAppTy g ity
if not (tyconRefEq g itcref g.system_GenericIComparable_tcref) &&
Expand All @@ -113,7 +120,18 @@ let rec GetImmediateInterfacesOfType g amap m typ =
yield Import.ImportProvidedType amap m ity ]
#endif
| ILTypeMetadata (scoref,tdef) ->
tdef.Implements |> ILList.toList |> List.map (ImportType scoref amap m tinst)

// ImportType may fail for an interface if the assembly load set is incomplete and the interface
// comes from another assembly. In this case we simply skip the interface:
// if we don't skip it, then compilation will just fail here, and if type checking
// succeeds with fewer non-dereferencable interfaces reported then it would have
// succeeded with more reported. There are pathological corner cases where this
// doesn't apply: e.g. for mscorlib interfaces like IComparable, but we can always
// assume those are present.
[ for ity in tdef.Implements |> ILList.toList do
if skipUnref = SkipUnrefInterfaces.No || CanImportType scoref amap m ity then
yield ImportType scoref amap m tinst ity ]

| FSharpOrArrayOrByrefOrTupleOrExnTypeMetadata ->
tcref.ImmediateInterfaceTypesOfFSharpTycon |> List.map (instType (mkInstForAppTy g typ))
else
Expand All @@ -133,7 +151,7 @@ type AllowMultiIntfInstantiations = Yes | No

/// Traverse the type hierarchy, e.g. f D (f C (f System.Object acc)).
/// Visit base types and interfaces first.
let private FoldHierarchyOfTypeAux followInterfaces allowMultiIntfInst visitor g amap m typ acc =
let private FoldHierarchyOfTypeAux followInterfaces allowMultiIntfInst skipUnref visitor g amap m typ acc =
let rec loop ndeep typ ((visitedTycon,visited:TyconRefMultiMap<_>,acc) as state) =

let seenThisTycon = isAppTy g typ && Set.contains (tcrefOfAppTy g typ).Stamp visitedTycon
Expand All @@ -157,7 +175,7 @@ let private FoldHierarchyOfTypeAux followInterfaces allowMultiIntfInst visitor g
if isInterfaceTy g typ then
List.foldBack
(loop (ndeep+1))
(GetImmediateInterfacesOfType g amap m typ)
(GetImmediateInterfacesOfType skipUnref g amap m typ)
(loop ndeep g.obj_ty state)
elif isTyparTy g typ then
let tp = destTyparTy g typ
Expand Down Expand Up @@ -186,7 +204,7 @@ let private FoldHierarchyOfTypeAux followInterfaces allowMultiIntfInst visitor g
if followInterfaces then
List.foldBack
(loop (ndeep+1))
(GetImmediateInterfacesOfType g amap m typ)
(GetImmediateInterfacesOfType skipUnref g amap m typ)
state
else
state
Expand All @@ -200,22 +218,25 @@ let private FoldHierarchyOfTypeAux followInterfaces allowMultiIntfInst visitor g
(visitedTycon,visited,acc)
loop 0 typ (Set.empty,TyconRefMultiMap<_>.Empty,acc) |> p33

/// Fold, do not follow interfaces
let FoldPrimaryHierarchyOfType f g amap m allowMultiIntfInst typ acc = FoldHierarchyOfTypeAux false allowMultiIntfInst f g amap m typ acc
/// Fold, do not follow interfaces (unless the type is itself an interface)
let FoldPrimaryHierarchyOfType f g amap m allowMultiIntfInst typ acc =
FoldHierarchyOfTypeAux false allowMultiIntfInst SkipUnrefInterfaces.No f g amap m typ acc

/// Fold, following interfaces
let FoldEntireHierarchyOfType f g amap m allowMultiIntfInst typ acc = FoldHierarchyOfTypeAux true allowMultiIntfInst f g amap m typ acc
/// Fold, following interfaces. Skipping interfaces that lie outside the referenced assembly set is allowed.
let FoldEntireHierarchyOfType f g amap m allowMultiIntfInst typ acc =
FoldHierarchyOfTypeAux true allowMultiIntfInst SkipUnrefInterfaces.Yes f g amap m typ acc

/// Iterate, following interfaces
let IterateEntireHierarchyOfType f g amap m allowMultiIntfInst typ = FoldHierarchyOfTypeAux true allowMultiIntfInst (fun ty () -> f ty) g amap m typ ()
/// Iterate, following interfaces. Skipping interfaces that lie outside the referenced assembly set is allowed.
let IterateEntireHierarchyOfType f g amap m allowMultiIntfInst typ =
FoldHierarchyOfTypeAux true allowMultiIntfInst SkipUnrefInterfaces.Yes (fun ty () -> f ty) g amap m typ ()

/// Search for one element satisfying a predicate, following interfaces
let ExistsInEntireHierarchyOfType f g amap m allowMultiIntfInst typ =
FoldHierarchyOfTypeAux true allowMultiIntfInst (fun ty acc -> acc || f ty ) g amap m typ false
FoldHierarchyOfTypeAux true allowMultiIntfInst SkipUnrefInterfaces.Yes (fun ty acc -> acc || f ty ) g amap m typ false

/// Search for one element where a function returns a 'Some' result, following interfaces
let SearchEntireHierarchyOfType f g amap m typ =
FoldHierarchyOfTypeAux true AllowMultiIntfInstantiations.Yes
FoldHierarchyOfTypeAux true AllowMultiIntfInstantiations.Yes SkipUnrefInterfaces.Yes
(fun ty acc ->
match acc with
| None -> if f ty then Some(ty) else None
Expand All @@ -224,7 +245,7 @@ let SearchEntireHierarchyOfType f g amap m typ =

/// Get all super types of the type, including the type itself
let AllSuperTypesOfType g amap m allowMultiIntfInst ty =
FoldHierarchyOfTypeAux true allowMultiIntfInst (ListSet.insert (typeEquiv g)) g amap m ty []
FoldHierarchyOfTypeAux true allowMultiIntfInst SkipUnrefInterfaces.No (ListSet.insert (typeEquiv g)) g amap m ty []

/// Get all interfaces of a type, including the type itself if it is an interface
let AllInterfacesOfType g amap m allowMultiIntfInst ty =
Expand Down
6 changes: 3 additions & 3 deletions src/fsharp/typrelns.fs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ let rec TypeDefinitelySubsumesTypeNoCoercion ndeep g amap m ty1 ty2 =
| Some ty -> TypeDefinitelySubsumesTypeNoCoercion (ndeep+1) g amap m ty1 ty) ||

(isInterfaceTy g ty1 &&
ty2 |> GetImmediateInterfacesOfType g amap m
ty2 |> GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m
|> List.exists (TypeDefinitelySubsumesTypeNoCoercion (ndeep+1) g amap m ty1))))


Expand Down Expand Up @@ -129,7 +129,7 @@ let rec TypeFeasiblySubsumesType ndeep g amap m ty1 canCoerce ty2 =
| None -> false
| Some ty -> TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce ty
end ||
ty2 |> GetImmediateInterfacesOfType g amap m
ty2 |> GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m
|> List.exists (TypeFeasiblySubsumesType (ndeep+1) g amap m ty1 NoCoerce))


Expand Down Expand Up @@ -1993,7 +1993,7 @@ let FinalTypeDefinitionChecksAtEndOfInferenceScope (infoReader:InfoReader) isImp
/// Look for the unique supertype of ty2 for which ty2 :> ty1 might feasibly hold
let FindUniqueFeasibleSupertype g amap m ty1 ty2 =
if not (isAppTy g ty2) then None else
let supertypes = Option.toList (GetSuperTypeOfType g amap m ty2) @ (GetImmediateInterfacesOfType g amap m ty2)
let supertypes = Option.toList (GetSuperTypeOfType g amap m ty2) @ (GetImmediateInterfacesOfType SkipUnrefInterfaces.Yes g amap m ty2)
supertypes |> List.tryFind (TypeFeasiblySubsumesType 0 g amap m ty1 NoCoerce)


Expand Down
4 changes: 3 additions & 1 deletion tests/fsharp/typecheck/sigs/build.bat
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@ REM Configure the sample, i.e. where to find the F# compiler and C# compiler.

call %~d0%~p0..\..\..\config.bat

"%FSC%" --noframework -r:"%FSCOREDLLPATH%" -r:"%X86_PROGRAMFILES%\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\mscorlib.dll" -r:"%X86_PROGRAMFILES%\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Core.dll" -r:"%X86_PROGRAMFILES%\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Data.dll" -r:"%X86_PROGRAMFILES%\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.dll" -r:"%X86_PROGRAMFILES%\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.5\System.Numerics.dll" -a -o:pos21.dll pos21.fs
@if ERRORLEVEL 1 goto Error

call ..\..\single-neg-test.bat neg91
@if ERRORLEVEL 1 goto Error

Expand Down Expand Up @@ -517,7 +520,6 @@ call ..\..\single-neg-test.bat neg35
"%FSC%" %fsc_flags% -a -o:pos05.dll pos05.fs
@if ERRORLEVEL 1 goto Error


:Ok
echo Built fsharp %~f0 ok.
endlocal
Expand Down
9 changes: 9 additions & 0 deletions tests/fsharp/typecheck/sigs/pos21.fs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
namespace Library2

open System
open System.Data

module M =
let dataset = new DataSet("test add reference")
Console.WriteLine(dataset.DataSetName)
Console.ReadKey(true)

0 comments on commit d52230b

Please sign in to comment.