Skip to content

Commit 5a8f454

Browse files
forkiTIHan
authored andcommitted
Use a MaxBuffer for Suggestions (#7060)
* Use MaxBuffer for Suggestions * Extract SuggestNames function * Rename Parameter * Apply feedback for formatting
1 parent 010bd00 commit 5a8f454

21 files changed

+402
-387
lines changed

src/fsharp/CompileOps.fs

Lines changed: 24 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,20 @@ let getErrorString key = SR.GetString key
600600

601601
let (|InvalidArgument|_|) (exn: exn) = match exn with :? ArgumentException as e -> Some e.Message | _ -> None
602602

603-
let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames: bool) =
603+
let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (canSuggestNames: bool) =
604+
605+
let suggestNames suggestionsF idText =
606+
if canSuggestNames then
607+
let buffer = ErrorResolutionHints.SuggestionBuffer idText
608+
if not buffer.Disabled then
609+
suggestionsF buffer.Add
610+
if not buffer.IsEmpty then
611+
os.Append " " |> ignore
612+
os.Append(FSComp.SR.undefinedNameSuggestionsIntro()) |> ignore
613+
for value in buffer do
614+
os.AppendLine() |> ignore
615+
os.Append " " |> ignore
616+
os.Append(DecompileOpName value) |> ignore
604617

605618
let rec OutputExceptionR (os: StringBuilder) error =
606619

@@ -822,14 +835,10 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames
822835

823836
| UndefinedName(_, k, id, suggestionsF) ->
824837
os.Append(k (DecompileOpName id.idText)) |> ignore
825-
if suggestNames then
826-
let filtered = ErrorResolutionHints.FilterPredictions suggestionsF id.idText
827-
if List.isEmpty filtered |> not then
828-
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore
829-
838+
suggestNames suggestionsF id.idText
830839

831840
| InternalUndefinedItemRef(f, smr, ccuName, s) ->
832-
let _, errs = f(smr, ccuName, s)
841+
let _, errs = f(smr, ccuName, s)
833842
os.Append errs |> ignore
834843

835844
| FieldNotMutable _ ->
@@ -1363,10 +1372,7 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames
13631372

13641373
| ErrorWithSuggestions ((_, s), _, idText, suggestionF) ->
13651374
os.Append(DecompileOpName s) |> ignore
1366-
if suggestNames then
1367-
let filtered = ErrorResolutionHints.FilterPredictions suggestionF idText
1368-
if List.isEmpty filtered |> not then
1369-
os.Append(ErrorResolutionHints.FormatPredictions DecompileOpName filtered) |> ignore
1375+
suggestNames suggestionF idText
13701376

13711377
| NumberedError ((_, s), _) -> os.Append s |> ignore
13721378

@@ -1582,10 +1588,10 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (suggestNames
15821588

15831589

15841590
// remove any newlines and tabs
1585-
let OutputPhasedDiagnostic (os: System.Text.StringBuilder) (err: PhasedDiagnostic) (flattenErrors: bool) (suggestNames: bool) =
1591+
let OutputPhasedDiagnostic (os: System.Text.StringBuilder) (err: PhasedDiagnostic) (flattenErrors: bool) (canSuggestNames: bool) =
15861592
let buf = new System.Text.StringBuilder()
15871593

1588-
OutputPhasedErrorR buf err suggestNames
1594+
OutputPhasedErrorR buf err canSuggestNames
15891595
let s = if flattenErrors then ErrorLogger.NormalizeErrorString (buf.ToString()) else buf.ToString()
15901596

15911597
os.Append s |> ignore
@@ -1635,7 +1641,7 @@ type Diagnostic =
16351641
| Long of bool * DiagnosticDetailedInfo
16361642

16371643
/// returns sequence that contains Diagnostic for the given error + Diagnostic for all related errors
1638-
let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err: PhasedDiagnostic, suggestNames: bool) =
1644+
let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err: PhasedDiagnostic, canSuggestNames: bool) =
16391645
let outputWhere (showFullPaths, errorStyle) m: DiagnosticLocation =
16401646
if Range.equals m rangeStartup || Range.equals m rangeCmdArgs then
16411647
{ Range = m; TextRepresentation = ""; IsEmpty = true; File = "" }
@@ -1708,7 +1714,7 @@ let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorSt
17081714
let canonical = OutputCanonicalInformation(err.Subcategory(), GetDiagnosticNumber mainError)
17091715
let message =
17101716
let os = System.Text.StringBuilder()
1711-
OutputPhasedDiagnostic os mainError flattenErrors suggestNames
1717+
OutputPhasedDiagnostic os mainError flattenErrors canSuggestNames
17121718
os.ToString()
17131719

17141720
let entry: DiagnosticDetailedInfo = { Location = where; Canonical = canonical; Message = message }
@@ -1723,15 +1729,15 @@ let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorSt
17231729
let relCanonical = OutputCanonicalInformation(err.Subcategory(), GetDiagnosticNumber mainError) // Use main error for code
17241730
let relMessage =
17251731
let os = System.Text.StringBuilder()
1726-
OutputPhasedDiagnostic os err flattenErrors suggestNames
1732+
OutputPhasedDiagnostic os err flattenErrors canSuggestNames
17271733
os.ToString()
17281734

17291735
let entry: DiagnosticDetailedInfo = { Location = relWhere; Canonical = relCanonical; Message = relMessage}
17301736
errors.Add( Diagnostic.Long (isError, entry) )
17311737

17321738
| _ ->
17331739
let os = System.Text.StringBuilder()
1734-
OutputPhasedDiagnostic os err flattenErrors suggestNames
1740+
OutputPhasedDiagnostic os err flattenErrors canSuggestNames
17351741
errors.Add( Diagnostic.Short(isError, os.ToString()) )
17361742

17371743
relatedErrors |> List.iter OutputRelatedError
@@ -1752,7 +1758,7 @@ let CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorSt
17521758
/// prints error and related errors to the specified StringBuilder
17531759
let rec OutputDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError) os (err: PhasedDiagnostic) =
17541760

1755-
// 'true' for "suggestNames" is passed last here because we want to report suggestions in fsc.exe and fsi.exe, just not in regular IDE usage.
1761+
// 'true' for "canSuggestNames" is passed last here because we want to report suggestions in fsc.exe and fsi.exe, just not in regular IDE usage.
17561762
let errors = CollectDiagnostic (implicitIncludeDir, showFullPaths, flattenErrors, errorStyle, isError, err, true)
17571763
for e in errors do
17581764
Printf.bprintf os "\n"

src/fsharp/ConstraintSolver.fs

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2193,12 +2193,11 @@ and ReportNoCandidatesError (csenv: ConstraintSolverEnv) (nUnnamedCallerArgs, nN
21932193
match cmeth.UnassignedNamedArgs with
21942194
| CallerNamedArg(id, _) :: _ ->
21952195
if minfo.IsConstructor then
2196-
let predictFields() =
2197-
minfo.DeclaringTyconRef.AllInstanceFieldsAsList
2198-
|> List.map (fun p -> p.Name.Replace("@", ""))
2199-
|> System.Collections.Generic.HashSet
2196+
let suggestFields (addToBuffer: string -> unit) =
2197+
for p in minfo.DeclaringTyconRef.AllInstanceFieldsAsList do
2198+
addToBuffer(p.Name.Replace("@", ""))
22002199

2201-
ErrorWithSuggestions((msgNum, FSComp.SR.csCtorHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange, id.idText, predictFields)
2200+
ErrorWithSuggestions((msgNum, FSComp.SR.csCtorHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange, id.idText, suggestFields)
22022201
else
22032202
Error((msgNum, FSComp.SR.csMemberHasNoArgumentOrReturnProperty(methodName, id.idText, msgText)), id.idRange)
22042203
| [] -> Error((msgNum, msgText), m)

src/fsharp/ErrorLogger.fs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,9 @@ let rec findOriginalException err =
4646
| WrappedError(err, _) -> findOriginalException err
4747
| _ -> err
4848

49-
type Suggestions = unit -> Collections.Generic.HashSet<string>
49+
type Suggestions = (string -> unit) -> unit
5050

51-
let NoSuggestions : Suggestions = fun () -> Collections.Generic.HashSet()
51+
let NoSuggestions : Suggestions = ignore
5252

5353
/// Thrown when we stop processing the F# Interactive entry or #load.
5454
exception StopProcessingExn of exn option with

src/fsharp/ErrorResolutionHints.fs

Lines changed: 76 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ module internal FSharp.Compiler.ErrorResolutionHints
55

66
open Internal.Utilities
77
open FSharp.Compiler.AbstractIL.Internal.Library
8+
open System.Collections
9+
open System.Collections.Generic
810

911
let maxSuggestions = 5
1012
let minThresholdForSuggestions = 0.7
1113
let highConfidenceThreshold = 0.85
12-
let minStringLengthForThreshold = 3
14+
let minStringLengthForSuggestion = 3
1315

1416
/// We report a candidate if its edit distance is <= the threshold.
1517
/// The threshold is set to about a quarter of the number of characters.
@@ -23,65 +25,82 @@ let IsInEditDistanceProximity idText suggestion =
2325

2426
editDistance <= threshold
2527

26-
/// Filters predictions based on edit distance to the given unknown identifier.
27-
let FilterPredictions (suggestionF:ErrorLogger.Suggestions) (idText:string) =
28+
/// Demangles a suggestion
29+
let DemangleOperator (nm: string) =
30+
if nm.StartsWithOrdinal("( ") && nm.EndsWithOrdinal(" )") then
31+
nm.[2..nm.Length - 3]
32+
else
33+
nm
34+
35+
type SuggestionBufferEnumerator(tail: int, data: KeyValuePair<float,string> []) =
36+
let mutable current = data.Length
37+
interface IEnumerator<string> with
38+
member __.Current
39+
with get () =
40+
let kvpr = &data.[current]
41+
kvpr.Value
42+
interface System.Collections.IEnumerator with
43+
member __.Current with get () = box data.[current].Value
44+
member __.MoveNext() =
45+
current <- current - 1
46+
current > tail || (current = tail && data.[current] <> Unchecked.defaultof<_>)
47+
member __.Reset () = current <- data.Length
48+
interface System.IDisposable with
49+
member __.Dispose () = ()
50+
51+
type SuggestionBuffer(idText: string) =
52+
let data = Array.zeroCreate<KeyValuePair<float,string>>(maxSuggestions)
53+
let mutable tail = maxSuggestions - 1
2854
let uppercaseText = idText.ToUpperInvariant()
29-
let allSuggestions = suggestionF()
55+
let dotIdText = "." + idText
56+
let mutable disableSuggestions = idText.Length < minStringLengthForSuggestion
3057

31-
let demangle (nm:string) =
32-
if nm.StartsWithOrdinal("( ") && nm.EndsWithOrdinal(" )") then
33-
let cleanName = nm.[2..nm.Length - 3]
34-
cleanName
35-
else nm
58+
let insert (k,v) =
59+
let mutable pos = tail
60+
while pos < maxSuggestions && (let kv = &data.[pos] in kv.Key < k) do
61+
pos <- pos + 1
3662

37-
/// Returns `true` if given string is an operator display name, e.g. ( |>> )
38-
let IsOperatorName (name: string) =
39-
if isNull name || not (name.StartsWithOrdinal("( ") && name.EndsWithOrdinal(" )")) then
40-
false
41-
else
42-
let mutable i = 2
43-
let mutable isOperator = true
44-
while isOperator && i < name.Length - 3 do
45-
if name.[i] = ' ' then
46-
isOperator <- false
47-
i <- i + 1
48-
isOperator
63+
if pos > 0 then
64+
if pos >= maxSuggestions || (let kv = &data.[pos] in k <> kv.Key || v <> kv.Value) then
65+
if tail < pos - 1 then
66+
for i = tail to pos - 2 do
67+
data.[i] <- data.[i + 1]
68+
data.[pos - 1] <- KeyValuePair(k,v)
69+
if tail > 0 then tail <- tail - 1
4970

50-
if allSuggestions.Contains idText then [] else // some other parsing error occurred
51-
let dotIdText = "." + idText
52-
allSuggestions
53-
|> Seq.choose (fun suggestion ->
54-
// Because beginning a name with _ is used both to indicate an unused
55-
// value as well as to formally squelch the associated compiler
56-
// error/warning (FS1182), we remove such names from the suggestions,
57-
// both to prevent accidental usages as well as to encourage good taste
58-
if IsOperatorName suggestion || suggestion.StartsWithOrdinal("_") then None else
59-
let suggestion:string = demangle suggestion
60-
let suggestedText = suggestion.ToUpperInvariant()
61-
let similarity = EditDistance.JaroWinklerDistance uppercaseText suggestedText
62-
if similarity >= highConfidenceThreshold || suggestion.EndsWithOrdinal(dotIdText) then
63-
Some(similarity, suggestion)
64-
elif similarity < minThresholdForSuggestions && suggestedText.Length > minStringLengthForThreshold then
65-
None
66-
elif IsInEditDistanceProximity uppercaseText suggestedText then
67-
Some(similarity, suggestion)
68-
else
69-
None)
70-
|> Seq.sortByDescending fst
71-
|> Seq.mapi (fun i x -> i, x)
72-
|> Seq.takeWhile (fun (i, _) -> i < maxSuggestions)
73-
|> Seq.map snd
74-
|> Seq.toList
71+
member __.Add (suggestion: string) =
72+
if not disableSuggestions then
73+
if suggestion = idText then // some other parse error happened
74+
disableSuggestions <- true
75+
76+
// Because beginning a name with _ is used both to indicate an unused
77+
// value as well as to formally squelch the associated compiler
78+
// error/warning (FS1182), we remove such names from the suggestions,
79+
// both to prevent accidental usages as well as to encourage good taste
80+
if suggestion.Length >= minStringLengthForSuggestion && not (suggestion.StartsWithOrdinal "_") then
81+
let suggestion:string = DemangleOperator suggestion
82+
let suggestedText = suggestion.ToUpperInvariant()
83+
let similarity = EditDistance.JaroWinklerDistance uppercaseText suggestedText
84+
if similarity >= highConfidenceThreshold ||
85+
suggestion.EndsWithOrdinal dotIdText ||
86+
(similarity >= minThresholdForSuggestions && IsInEditDistanceProximity uppercaseText suggestedText)
87+
then
88+
insert(similarity, suggestion) |> ignore
89+
90+
member __.Disabled with get () = disableSuggestions
91+
92+
member __.IsEmpty with get () = disableSuggestions || (tail = maxSuggestions - 1)
7593

76-
/// Formats the given predictions according to the error style.
77-
let FormatPredictions normalizeF (predictions: (float * string) list) =
78-
match predictions with
79-
| [] -> System.String.Empty
80-
| _ ->
81-
let suggestions =
82-
predictions
83-
|> List.map (snd >> normalizeF)
84-
|> List.map (sprintf "%s %s" System.Environment.NewLine)
85-
|> String.concat ""
94+
interface IEnumerable<string> with
95+
member this.GetEnumerator () =
96+
if this.IsEmpty then
97+
Seq.empty.GetEnumerator()
98+
else
99+
new SuggestionBufferEnumerator(tail, data) :> IEnumerator<string>
86100

87-
" " + FSComp.SR.undefinedNameSuggestionsIntro() + suggestions
101+
interface IEnumerable with
102+
member this.GetEnumerator () =
103+
if this.IsEmpty then
104+
Seq.empty.GetEnumerator() :> IEnumerator
105+
else
106+
new SuggestionBufferEnumerator(tail, data) :> IEnumerator

0 commit comments

Comments
 (0)