Skip to content

Commit 26711a7

Browse files
committed
Suggest a similarly-spelled identifier on undefined-identifier errors
Add a 'did you mean ...?' note to E30015 when a close in-scope name exists, using a new StringUtil::calcLevenshteinDistance helper. Conservative: scales the allowed edit distance with name length and excludes core-module builtins so it stays quiet in normal code. Fixes #9124.
1 parent a214f7d commit 26711a7

8 files changed

Lines changed: 196 additions & 3 deletions

File tree

source/core/slang-string-util.cpp

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -975,4 +975,41 @@ int StringUtil::parseIntAndAdvancePos(UnownedStringSlice text, Index& pos)
975975
return result;
976976
}
977977

978+
Index StringUtil::calcLevenshteinDistance(const UnownedStringSlice& a, const UnownedStringSlice& b)
979+
{
980+
const Index lenA = a.getLength();
981+
const Index lenB = b.getLength();
982+
if (lenA == 0)
983+
return lenB;
984+
if (lenB == 0)
985+
return lenA;
986+
987+
// Classic dynamic-programming Levenshtein using two rolling rows so the
988+
// working set is O(lenB) rather than O(lenA * lenB).
989+
List<Index> prevRow;
990+
List<Index> currRow;
991+
prevRow.setCount(lenB + 1);
992+
currRow.setCount(lenB + 1);
993+
994+
for (Index j = 0; j <= lenB; ++j)
995+
prevRow[j] = j;
996+
997+
for (Index i = 1; i <= lenA; ++i)
998+
{
999+
currRow[0] = i;
1000+
const char ca = a[i - 1];
1001+
for (Index j = 1; j <= lenB; ++j)
1002+
{
1003+
const Index cost = (ca == b[j - 1]) ? 0 : 1;
1004+
const Index deletion = prevRow[j] + 1;
1005+
const Index insertion = currRow[j - 1] + 1;
1006+
const Index substitution = prevRow[j - 1] + cost;
1007+
currRow[j] = Math::Min(deletion, insertion, substitution);
1008+
}
1009+
prevRow.swapWith(currRow);
1010+
}
1011+
1012+
return prevRow[lenB];
1013+
}
1014+
9781015
} // namespace Slang

source/core/slang-string-util.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,13 @@ struct StringUtil
198198
/// Modifies pos to the position where parsing ends.
199199
/// Returns parsed integer.
200200
static int parseIntAndAdvancePos(UnownedStringSlice text, Index& pos);
201+
202+
/// Compute the Levenshtein edit distance between `a` and `b`: the minimum number of
203+
/// single-character insertions, deletions, or substitutions needed to turn one string
204+
/// into the other. Used, for example, to suggest the closest known identifier for a
205+
/// misspelled name ("did you mean ...?"). The comparison is case-sensitive; lower-case
206+
/// the inputs beforehand for a case-insensitive distance.
207+
static Index calcLevenshteinDistance(const UnownedStringSlice& a, const UnownedStringSlice& b);
201208
};
202209

203210
/* A helper class that allows parsing of lines from text with iteration. Uses

source/slang/slang-check-expr.cpp

Lines changed: 94 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
// * `slang-check-conversion.cpp` is responsible for the logic of handling type conversion/coercion
1313

1414
#include "../core/slang-math.h"
15+
#include "../core/slang-string-util.h"
1516
#include "core/slang-char-util.h"
1617
#include "slang-ast-decl.h"
1718
#include "slang-ast-natural-layout.h"
@@ -5053,6 +5054,87 @@ Expr* SemanticsExprVisitor::visitInvokeExpr(InvokeExpr* expr)
50535054
return checkedExpr;
50545055
}
50555056

5057+
// Find the in-scope identifier whose spelling is closest to `name`, to power a
5058+
// "did you mean ...?" suggestion when `name` failed to resolve. Walks the scope
5059+
// chain (and each scope's sibling chain) collecting the names of direct members,
5060+
// computes the case-insensitive Levenshtein distance to each, and returns the
5061+
// single closest candidate within a small distance threshold. Returns nullptr if
5062+
// nothing is close enough, so the caller can simply omit the suggestion. Only
5063+
// direct members are considered (not inherited/extension members); that is the
5064+
// common typo case and keeps this off the hot path of successful lookups.
5065+
static Name* findClosestInScopeName(Name* name, Scope* scope)
5066+
{
5067+
if (!name)
5068+
return nullptr;
5069+
5070+
const UnownedStringSlice target = getUnownedStringSliceText(name);
5071+
if (target.getLength() == 0)
5072+
return nullptr;
5073+
5074+
// Lower-case the target once for a case-insensitive comparison.
5075+
StringBuilder targetLowerBuilder;
5076+
for (auto c : target)
5077+
targetLowerBuilder.appendChar(CharUtil::toLower(c));
5078+
const String targetLower = targetLowerBuilder.produceString();
5079+
5080+
// Scale the allowed edit distance with the identifier length and cap it, so
5081+
// that we only offer genuinely-close names (e.g. `lenght` -> `length`, or
5082+
// `f_a` -> `f_b`) and never a wildly different one (e.g. the keyword `case`
5083+
// -> the module `core`, a distance-2 edit on a 4-char name). Roughly one edit
5084+
// per three characters with a floor of one, matching the heuristic used by
5085+
// other compilers. Names shorter than 3 chars are too short to suggest against.
5086+
if (target.getLength() < 3)
5087+
return nullptr;
5088+
const Index maxDistance = Math::Min<Index>(3, Math::Max<Index>(1, target.getLength() / 3));
5089+
5090+
Name* best = nullptr;
5091+
Index bestDistance = maxDistance + 1;
5092+
5093+
for (Scope* s = scope; s; s = s->parent)
5094+
{
5095+
for (Scope* sib = s; sib; sib = sib->nextSibling)
5096+
{
5097+
auto containerDecl = sib->containerDecl;
5098+
if (!containerDecl)
5099+
continue;
5100+
5101+
for (auto candidateDecl : containerDecl->getDirectMemberDecls())
5102+
{
5103+
Name* candidateName = candidateDecl->getName();
5104+
if (!candidateName || candidateName == name)
5105+
continue;
5106+
5107+
// Don't suggest names from the core module. Its global scope holds
5108+
// thousands of builtins, so almost any identifier finds a spurious
5109+
// close match there (e.g. `instance` -> `distance`); restricting to
5110+
// user-written declarations keeps suggestions quiet and relevant.
5111+
if (isFromCoreModule(candidateDecl))
5112+
continue;
5113+
5114+
const UnownedStringSlice candidateText = getUnownedStringSliceText(candidateName);
5115+
if (candidateText.getLength() == 0)
5116+
continue;
5117+
5118+
StringBuilder candidateLowerBuilder;
5119+
for (auto c : candidateText)
5120+
candidateLowerBuilder.appendChar(CharUtil::toLower(c));
5121+
const String candidateLower = candidateLowerBuilder.produceString();
5122+
5123+
const Index distance = StringUtil::calcLevenshteinDistance(
5124+
targetLower.getUnownedSlice(),
5125+
candidateLower.getUnownedSlice());
5126+
if (distance < bestDistance)
5127+
{
5128+
bestDistance = distance;
5129+
best = candidateName;
5130+
}
5131+
}
5132+
}
5133+
}
5134+
5135+
return (bestDistance <= maxDistance) ? best : nullptr;
5136+
}
5137+
50565138
Expr* SemanticsExprVisitor::visitVarExpr(VarExpr* expr)
50575139
{
50585140
// If we've already resolved this expression, don't try again.
@@ -5097,8 +5179,18 @@ Expr* SemanticsExprVisitor::visitVarExpr(VarExpr* expr)
50975179
}
50985180

50995181
if (!diagnosed)
5100-
getSink()->diagnose(
5101-
Diagnostics::UndefinedIdentifier{.name = expr->name, .location = expr->loc});
5182+
{
5183+
// If a similarly-spelled identifier is in scope, attach a "did you mean
5184+
// 'length'?" note to the error; this turns a bare "undefined identifier
5185+
// 'lenght'" into an actionable hint. The note only renders when
5186+
// `suggestionLocation` is valid, so a null suggestion is harmless.
5187+
auto suggestion = findClosestInScopeName(expr->name, expr->scope);
5188+
getSink()->diagnose(Diagnostics::UndefinedIdentifier{
5189+
.name = expr->name,
5190+
.suggestion = suggestion,
5191+
.location = expr->loc,
5192+
.suggestionLocation = suggestion ? expr->loc : SourceLoc{}});
5193+
}
51025194

51035195
return resultExpr;
51045196
}

source/slang/slang-diagnostics.lua

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1130,7 +1130,10 @@ err(
11301130
"undefined-identifier",
11311131
30015,
11321132
"undefined identifier",
1133-
span { loc = "location", message = "undefined identifier '~name:Name'." }
1133+
span { loc = "location", message = "undefined identifier '~name:Name'." },
1134+
-- Optional "did you mean ...?" note, rendered only when `suggestionLocation`
1135+
-- is set to a valid location by the caller (i.e. a close in-scope name exists).
1136+
note { message = "did you mean '~suggestion:Name'?", span { loc = "suggestionLocation" } }
11341137
)
11351138

11361139
err(

tests/diagnostics/autodiff-custom-diff-unresolved.slang

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ void __d_f(float x, float.Differential dout)
1212
/*CHECK:
1313
^^^^^ undefined identifier
1414
^^^^^ undefined identifier '__d_g'.
15+
^^^^^ did you mean '__d_f'?
1516
^^^^^^^^^^^^^^^^^^ cannot resolve derivative function
1617
^^^^^^^^^^^^^^^^^^ cannot resolve the custom derivative function
1718
*/
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Test for issue #9124: an undefined identifier that is a close misspelling of
2+
// an in-scope name produces a "did you mean ...?" suggestion note.
3+
4+
//DIAGNOSTIC_TEST:SIMPLE(diag=CHECK):-target spirv
5+
6+
float closeMatch(float myVariable)
7+
{
8+
return myVariabl;
9+
//CHECK: ^^^^^^^^^ undefined identifier
10+
//CHECK: ^^^^^^^^^ undefined identifier 'myVariabl'.
11+
//CHECK: ^^^^^^^^^ did you mean 'myVariable'?
12+
}
13+
14+
// A name with no nearby candidate must NOT produce a suggestion note.
15+
float noMatch()
16+
{
17+
return zzzzqqq;
18+
//CHECK: ^^^^^^^ undefined identifier
19+
//CHECK: ^^^^^^^ undefined identifier 'zzzzqqq'.
20+
}

tests/diagnostics/transitive-namespace-import.slang/test.slang

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ namespace ns
2222
f_a(); // Error.
2323
//CHECK:^^^ undefined identifier
2424
//CHECK:^^^ undefined identifier 'f_a'.
25+
//CHECK:^^^ did you mean 'f_b'?
2526
}
2627
}
2728

@@ -35,5 +36,6 @@ namespace ns2
3536
f_a(); // Error.
3637
//CHECK:^^^ undefined identifier
3738
//CHECK:^^^ undefined identifier 'f_a'.
39+
//CHECK:^^^ did you mean 'f_b'?
3840
}
3941
}

tools/slang-unit-test/unit-test-string-util.cpp

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -296,3 +296,34 @@ SLANG_UNIT_TEST(stringUtilAppendFormat)
296296
StringUtil::appendFormat(buf, " v=%d", 7);
297297
SLANG_CHECK(buf.produceString() == "prefix: v=7");
298298
}
299+
300+
// -- Levenshtein edit distance ----------------------------------------
301+
302+
SLANG_UNIT_TEST(stringUtilLevenshteinDistance)
303+
{
304+
// Identical strings have distance 0.
305+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice("length"), toSlice("length")) == 0);
306+
307+
// Either side empty: distance is the length of the other.
308+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice(""), toSlice("abc")) == 3);
309+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice("abc"), toSlice("")) == 3);
310+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice(""), toSlice("")) == 0);
311+
312+
// Single insertion / deletion.
313+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice("color"), toSlice("colour")) == 1);
314+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice("colour"), toSlice("color")) == 1);
315+
// Single substitution.
316+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice("cat"), toSlice("cot")) == 1);
317+
// Two adjacent substitutions (`in` -> `di`).
318+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice("instance"), toSlice("distance")) == 2);
319+
// A transposition counts as two edits under plain Levenshtein.
320+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice("lenght"), toSlice("length")) == 2);
321+
322+
// Classic textbook example.
323+
SLANG_CHECK(StringUtil::calcLevenshteinDistance(toSlice("kitten"), toSlice("sitting")) == 3);
324+
325+
// Symmetry.
326+
SLANG_CHECK(
327+
StringUtil::calcLevenshteinDistance(toSlice("abcdef"), toSlice("azced")) ==
328+
StringUtil::calcLevenshteinDistance(toSlice("azced"), toSlice("abcdef")));
329+
}

0 commit comments

Comments
 (0)