Skip to content

Commit

Permalink
Use a MaxBuffer for Suggestions
Browse files Browse the repository at this point in the history
  • Loading branch information
forki committed Jun 25, 2019
1 parent 6633abe commit c5cc18e
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 75 deletions.
4 changes: 2 additions & 2 deletions src/fsharp/CompileOps.fs
Original file line number Diff line number Diff line change
Expand Up @@ -827,7 +827,7 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames
os.Append(k (DecompileOpName id.idText)) |> ignore
if suggestNames then
let filtered = ErrorResolutionHints.FilterPredictions suggestionsF id.idText
if List.isEmpty filtered |> not then
if Array.isEmpty filtered |> not then
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore


Expand Down Expand Up @@ -1368,7 +1368,7 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames
os.Append(DecompileOpName s) |> ignore
if suggestNames then
let filtered = ErrorResolutionHints.FilterPredictions suggestionF idText
if List.isEmpty filtered |> not then
if Array.isEmpty filtered |> not then
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore

| NumberedError ((_, s), _) -> os.Append s |> ignore
Expand Down
3 changes: 1 addition & 2 deletions src/fsharp/ConstraintSolver.fs
Original file line number Diff line number Diff line change
Expand Up @@ -2195,8 +2195,7 @@ and ReportNoCandidatesError (csenv: ConstraintSolverEnv) (nUnnamedCallerArgs, nN
if minfo.IsConstructor then
let predictFields() =
minfo.DeclaringTyconRef.AllInstanceFieldsAsList
|> List.map (fun p -> p.Name.Replace("@", ""))
|> System.Collections.Generic.HashSet
|> Seq.map (fun p -> p.Name.Replace("@", ""))

ErrorWithSuggestions((msgNum, FSComp.SR.csCtorHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange, id.idText, predictFields)
else
Expand Down
4 changes: 2 additions & 2 deletions src/fsharp/ErrorLogger.fs
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,9 @@ let rec findOriginalException err =
| WrappedError(err, _) -> findOriginalException err
| _ -> err

type Suggestions = unit -> Collections.Generic.HashSet<string>
type Suggestions = unit -> seq<string>

let NoSuggestions : Suggestions = fun () -> Collections.Generic.HashSet()
let NoSuggestions : Suggestions = fun () -> Seq.empty

/// Thrown when we stop processing the F# Interactive entry or #load.
exception StopProcessingExn of exn option with
Expand Down
90 changes: 61 additions & 29 deletions src/fsharp/ErrorResolutionHints.fs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,39 @@ let minThresholdForSuggestions = 0.7
let highConfidenceThreshold = 0.85
let minStringLengthForThreshold = 3

type MaxBuffer<'K,'V when 'K : comparison and 'V : equality>(max:int) =
let data = Array.zeroCreate<System.Collections.Generic.KeyValuePair<'K,'V voption>>(max)
let rec backup i j =
if i < j then
data.[i] <- data.[i + 1]
backup (i + 1) j

let rec find i k v =
let kvpr : byref<_> = &data.[i] //only want key (not KVP)
let kr = kvpr.Key
if k < kr then
if i = 0 then
false
else
backup 0 (i-1)
data.[i-1] <- System.Collections.Generic.KeyValuePair<_,_>(k, ValueSome v)
true
else
if i = max - 1 then // end of the line, replace at head
backup 0 i
data.[i] <- System.Collections.Generic.KeyValuePair<_,_>(k, ValueSome v)
true
else
find (i + 1) k v
member __.Insert (k,v) = find 0 k v
member __.Values() : 'V [] =
let hashSet = System.Collections.Generic.HashSet<'V>()
for kv in data do
match kv.Value with
| ValueSome x -> hashSet.Add x |> ignore
| _ -> ()
hashSet |> Seq.toArray

/// We report a candidate if its edit distance is <= the threshold.
/// The threshold is set to about a quarter of the number of characters.
let IsInEditDistanceProximity idText suggestion =
Expand Down Expand Up @@ -42,41 +75,40 @@ let FilterPredictions (suggestionF:ErrorLogger.Suggestions) (idText:string) =
let name = name.[2..name.Length - 3]
name |> Seq.forall (fun c -> c <> ' ')

if allSuggestions.Contains idText then [] else // some other parsing error occurred
let dotIdText = "." + idText
allSuggestions
|> Seq.choose (fun suggestion ->
// Because beginning a name with _ is used both to indicate an unused
// value as well as to formally squelch the associated compiler
// error/warning (FS1182), we remove such names from the suggestions,
// both to prevent accidental usages as well as to encourage good taste
if IsOperatorName suggestion || suggestion.StartsWithOrdinal("_") then None else
let suggestion:string = demangle suggestion
let suggestedText = suggestion.ToUpperInvariant()
let similarity = EditDistance.JaroWinklerDistance uppercaseText suggestedText
if similarity >= highConfidenceThreshold || suggestion.EndsWithOrdinal(dotIdText) then
Some(similarity, suggestion)
elif similarity < minThresholdForSuggestions && suggestedText.Length > minStringLengthForThreshold then
None
elif IsInEditDistanceProximity uppercaseText suggestedText then
Some(similarity, suggestion)
let mutable parsingError = false
let buffer = MaxBuffer(maxSuggestions)
for suggestion in allSuggestions do
if parsingError || suggestion = idText then
parsingError <- true // some other parsing error occurred
else
None)
|> Seq.sortByDescending fst
|> Seq.mapi (fun i x -> i, x)
|> Seq.takeWhile (fun (i, _) -> i < maxSuggestions)
|> Seq.map snd
|> Seq.toList
// Because beginning a name with _ is used both to indicate an unused
// value as well as to formally squelch the associated compiler
// error/warning (FS1182), we remove such names from the suggestions,
// both to prevent accidental usages as well as to encourage good taste
if IsOperatorName suggestion || suggestion.StartsWithOrdinal("_") then
()
else
let suggestion:string = demangle suggestion
let suggestedText = suggestion.ToUpperInvariant()
let similarity = EditDistance.JaroWinklerDistance uppercaseText suggestedText
if similarity >= highConfidenceThreshold || suggestion.EndsWithOrdinal(dotIdText) then
buffer.Insert(similarity, suggestion) |> ignore
elif similarity < minThresholdForSuggestions && suggestedText.Length > minStringLengthForThreshold then
()
elif IsInEditDistanceProximity uppercaseText suggestedText then
buffer.Insert(similarity, suggestion) |> ignore

if parsingError then [||] else buffer.Values()

/// Formats the given predictions according to the error style.
let FormatPredictions normalizeF (predictions: (float * string) list) =
match predictions with
| [] -> System.String.Empty
| _ ->
let FormatPredictions normalizeF (predictions: #seq<string>) =
if Seq.isEmpty predictions then
System.String.Empty
else
let suggestions =
predictions
|> List.map (snd >> normalizeF)
|> List.map (sprintf "%s %s" System.Environment.NewLine)
|> Seq.map (fun p -> sprintf "%s %s" System.Environment.NewLine (normalizeF p))
|> String.concat ""

" " + FSComp.SR.undefinedNameSuggestionsIntro() + suggestions
48 changes: 17 additions & 31 deletions src/fsharp/NameResolution.fs
Original file line number Diff line number Diff line change
Expand Up @@ -899,9 +899,7 @@ let AddResults res1 res2 =
| Result x, Exception _ -> Result x
// If we have error messages for the same symbol, then we can merge suggestions.
| Exception (UndefinedName(n1, f, id1, suggestions1)), Exception (UndefinedName(n2, _, id2, suggestions2)) when n1 = n2 && id1.idText = id2.idText && Range.equals id1.idRange id2.idRange ->
let suggestions = HashSet(suggestions1())
suggestions.UnionWith(suggestions2())
Exception(UndefinedName(n1, f, id1, fun () -> suggestions))
Exception(UndefinedName(n1, f, id1, fun () -> Seq.append (suggestions1()) (suggestions2())))
// This prefers error messages coming from deeper failing long identifier paths
| Exception (UndefinedName(n1, _, _, _) as e1), Exception (UndefinedName(n2, _, _, _) as e2) ->
if n1 < n2 then Exception e2 else Exception e1
Expand Down Expand Up @@ -1845,7 +1843,6 @@ let rec ResolveLongIndentAsModuleOrNamespace sink atMostOne amap m first fullyQu
|> Seq.collect (fun kv -> kv.Value)
|> Seq.filter (fun modref -> IsEntityAccessible amap m ad modref)
|> Seq.collect (fun e -> [e.DisplayName; e.DemangledModuleOrNamespaceName])
|> HashSet

UndefinedName(0, FSComp.SR.undefinedNameNamespaceOrModule, id, suggestModulesAndNamespaces))

Expand All @@ -1858,7 +1855,6 @@ let rec ResolveLongIndentAsModuleOrNamespace sink atMostOne amap m first fullyQu
mty.ModulesAndNamespacesByDemangledName
|> Seq.filter (fun kv -> IsEntityAccessible amap m ad (modref.NestedTyconRef kv.Value))
|> Seq.collect (fun e -> [e.Value.DisplayName; e.Value.DemangledModuleOrNamespaceName])
|> HashSet

let error = raze (UndefinedName(depth, FSComp.SR.undefinedNameNamespace, id, suggestNames))
moduleNotFoundErrorCache <- Some(id.idRange, error)
Expand Down Expand Up @@ -2260,13 +2256,13 @@ let rec ResolveLongIdentInTypePrim (ncenv: NameResolver) nenv lookupKind (resInf
[||]
| _ -> [||]

[ yield! suggestions1
seq {
yield! suggestions1
yield! suggestions2
yield! suggestions3
yield! suggestions4
yield! suggestions5
yield! suggestions6 ]
|> HashSet
yield! suggestions6 }

raze (UndefinedName (depth, FSComp.SR.undefinedNameFieldConstructorOrMember, id, suggestMembers))

Expand Down Expand Up @@ -2411,12 +2407,12 @@ let rec ResolveExprLongIdentInModuleOrNamespace (ncenv: NameResolver) nenv (type
|> Seq.filter (fun e -> IsTyconReprAccessible ncenv.amap m ad (modref.NestedTyconRef e.Value))
|> Seq.map (fun e -> e.Value.DisplayName)

[ yield! types
seq{
yield! types
yield! submodules
yield! unions
yield! vals
yield! exns ]
|> HashSet
yield! exns }

raze (UndefinedName(depth, FSComp.SR.undefinedNameValueConstructorNamespaceOrType, id, suggestPossibleTypesAndNames))
| results -> AtMostOneResult id.idRange results
Expand Down Expand Up @@ -2534,11 +2530,11 @@ let rec ResolveExprLongIdentPrim sink (ncenv: NameResolver) first fullyQualified
None)
|> Seq.map (fun t -> t.DisplayName + "." + id.idText)

[ yield! suggestedNames
seq {
yield! suggestedNames
yield! suggestedTypes
yield! suggestedModulesAndNamespaces
yield! unions ]
|> HashSet
yield! unions }

raze (UndefinedName(0, FSComp.SR.undefinedNameValueOfConstructor, id, suggestNamesAndTypes))
ForceRaise failingCase
Expand Down Expand Up @@ -2620,14 +2616,11 @@ let rec ResolveExprLongIdentPrim sink (ncenv: NameResolver) first fullyQualified
yield!
nenv.eUnqualifiedItems
|> Seq.map (fun e -> e.Value.DisplayName)
} |> HashSet
}

match innerSearch with
| Exception (UndefinedName(0, _, id1, suggestionsF)) when Range.equals id.idRange id1.idRange ->
let mergeSuggestions() =
let res = suggestEverythingInScope()
res.UnionWith(suggestionsF())
res
let mergeSuggestions() = Seq.append (suggestionsF()) (suggestEverythingInScope())

let failingCase = raze (UndefinedName(0, FSComp.SR.undefinedNameValueNamespaceTypeOrModule, id, mergeSuggestions))
ForceRaise failingCase
Expand Down Expand Up @@ -2720,9 +2713,9 @@ let rec ResolvePatternLongIdentInModuleOrNamespace (ncenv: NameResolver) nenv nu
|> Seq.filter (fun e -> IsEntityAccessible ncenv.amap m ad e.Value)
|> Seq.map (fun e -> e.Value.DisplayName)

[ yield! submodules
yield! suggestedTypes ]
|> HashSet
seq {
yield! submodules
yield! suggestedTypes }

raze (UndefinedName(depth, FSComp.SR.undefinedNameConstructorModuleOrNamespace, id, suggestPossibleTypes))
| results -> AtMostOneResult id.idRange results
Expand Down Expand Up @@ -2831,7 +2824,6 @@ let rec ResolveTypeLongIdentInTyconRefPrim (ncenv: NameResolver) (typeNameResInf
let suggestTypes() =
tcref.ModuleOrNamespaceType.TypesByDemangledNameAndArity id.idRange
|> Seq.map (fun e -> e.Value.DisplayName)
|> HashSet

raze (UndefinedName(depth, FSComp.SR.undefinedNameType, id, suggestTypes))
| id2 :: rest2 ->
Expand All @@ -2852,7 +2844,6 @@ let rec ResolveTypeLongIdentInTyconRefPrim (ncenv: NameResolver) (typeNameResInf
let suggestTypes() =
tcref.ModuleOrNamespaceType.TypesByDemangledNameAndArity id.idRange
|> Seq.map (fun e -> e.Value.DisplayName)
|> HashSet

raze (UndefinedName(depth, FSComp.SR.undefinedNameType, id, suggestTypes))

Expand All @@ -2877,7 +2868,6 @@ let SuggestTypeLongIdentInModuleOrNamespace depth (modref: ModuleOrNamespaceRef)
modref.ModuleOrNamespaceType.AllEntities
|> Seq.filter (fun e -> IsEntityAccessible amap m ad (modref.NestedTyconRef e))
|> Seq.collect (fun e -> [e.DisplayName; e.DemangledModuleOrNamespaceName])
|> HashSet

let errorTextF s = FSComp.SR.undefinedNameTypeIn(s, fullDisplayTextOfModRef modref)
UndefinedName(depth, errorTextF, id, suggestPossibleTypes)
Expand Down Expand Up @@ -2905,7 +2895,6 @@ let rec private ResolveTypeLongIdentInModuleOrNamespace sink nenv (ncenv: NameRe
modref.ModuleOrNamespaceType.ModulesAndNamespacesByDemangledName
|> Seq.filter (fun kv -> IsEntityAccessible ncenv.amap m ad (modref.NestedTyconRef kv.Value))
|> Seq.collect (fun e -> [e.Value.DisplayName; e.Value.DemangledModuleOrNamespaceName])
|> HashSet
raze (UndefinedName(depth, FSComp.SR.undefinedNameNamespaceOrModule, id, suggestPossibleModules))

let tyconSearch =
Expand All @@ -2916,7 +2905,6 @@ let rec private ResolveTypeLongIdentInModuleOrNamespace sink nenv (ncenv: NameRe
let suggestTypes() =
modref.ModuleOrNamespaceType.TypesByDemangledNameAndArity id.idRange
|> Seq.map (fun e -> e.Value.DisplayName)
|> HashSet

raze (UndefinedName(depth, FSComp.SR.undefinedNameType, id, suggestTypes))

Expand Down Expand Up @@ -2959,7 +2947,6 @@ let rec ResolveTypeLongIdentPrim sink (ncenv: NameResolver) occurence first full
if e.Value.DisplayName.EndsWithOrdinal("Attribute") then
yield e.Value.DisplayName.Replace("Attribute", "")]
| _ -> [e.Value.DisplayName; e.Value.DemangledModuleOrNamespaceName])
|> HashSet

raze (UndefinedName(0, FSComp.SR.undefinedNameType, id, suggestPossibleTypes))
| id2 :: rest2 ->
Expand Down Expand Up @@ -3116,7 +3103,7 @@ let SuggestLabelsOfRelatedRecords g (nenv: NameResolutionEnv) (id: Ident) (allFi
labelsOfPossibleRecords.ExceptWith givenFields
labelsOfPossibleRecords

if fullyQualfied.Count > 0 then fullyQualfied else
if fullyQualfied.Count > 0 then fullyQualfied :> seq<_> else

// check if the user forgot to use qualified access
nenv.eTyconsByDemangledNameAndArity
Expand All @@ -3130,7 +3117,6 @@ let SuggestLabelsOfRelatedRecords g (nenv: NameResolutionEnv) (id: Ident) (allFi
else
None)
|> Seq.map (fun t -> t.DisplayName + "." + id.idText)
|> HashSet

UndefinedName(0, FSComp.SR.undefinedNameRecordLabel, id, suggestLabels)

Expand Down Expand Up @@ -3159,7 +3145,7 @@ let ResolveFieldPrim sink (ncenv: NameResolver) nenv ad ty (mp, id: Ident) allFi
| _ ->
if isRecdTy g ty then
// record label doesn't belong to record type -> suggest other labels of same record
let suggestLabels() = SuggestOtherLabelsOfSameRecordType g nenv ty id allFields
let suggestLabels() = SuggestOtherLabelsOfSameRecordType g nenv ty id allFields :> seq<_>
let typeName = NicePrint.minimalStringOfType nenv.eDisplayEnv ty
let errorText = FSComp.SR.nrRecordDoesNotContainSuchLabel(typeName, id.idText)
error(ErrorWithSuggestions(errorText, m, id.idText, suggestLabels))
Expand Down
9 changes: 4 additions & 5 deletions src/fsharp/TypeChecker.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4555,9 +4555,9 @@ and TcTyparOrMeasurePar optKind cenv (env: TcEnv) newOk tpenv (Typar(id, _, _) a
elements
|> Seq.map (fun p -> "'" + p.Key)

[ yield! predictions1
yield! predictions2 ]
|> HashSet
seq {
yield! predictions1
yield! predictions2 }

let reportedId = Ident("'" + id.idText, id.idRange)
error (UndefinedName(0, FSComp.SR.undefinedNameTypeParameter, reportedId, predictTypeParameters))
Expand Down Expand Up @@ -6550,8 +6550,7 @@ and FreshenObjExprAbstractSlot cenv (env: TcEnv) (implty: TType) virtNameAndArit

let suggestVirtualMembers() =
virtNameAndArityPairs
|> List.map (fst >> fst)
|> HashSet
|> Seq.map (fst >> fst)

if containsNonAbstractMemberWithSameName then
errorR(ErrorWithSuggestions(FSComp.SR.tcMemberFoundIsNotAbstractOrVirtual(tcref.DisplayName, bindName), mBinding, bindName, suggestVirtualMembers))
Expand Down
9 changes: 5 additions & 4 deletions src/fsharp/service/ServiceErrorResolutionHints.fs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ open FSharp.Compiler.ErrorResolutionHints

module ErrorResolutionHints =
let getSuggestedNames (namesToCheck: string[]) (unresolvedIdentifier: string) =
let res = FilterPredictions (fun () -> HashSet<string>(namesToCheck)) unresolvedIdentifier |> List.map snd
match res with
| [] -> None
| _ -> Some res
let res = FilterPredictions (fun () -> namesToCheck :> seq<_>) unresolvedIdentifier
if Seq.isEmpty res then
None
else
Some(Array.toList res)

0 comments on commit c5cc18e

Please sign in to comment.