Suggest a similarly-spelled identifier on undefined-identifier errors (#9124) by expipiplus1 · Pull Request #11624 · shader-slang/slang

expipiplus1 · 2026-06-16T08:50:47Z

Adds a "did you mean ''?" note to the undefined-identifier error (E30015) when a close in-scope name exists. The match is conservative to avoid noise: the allowed edit distance scales with the identifier length (capped at 3, minimum length 3) and core-module builtins are excluded. Adds StringUtil::calcLevenshteinDistance with unit tests, and attaches the suggestion as an optional note on the existing diagnostic rather than a detached standalone note.

Fixes #9124.

coderabbitai · 2026-06-16T08:51:04Z

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

@coderabbitai resume to resume automatic reviews.
@coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

▶️ Resume reviews
🔍 Trigger review

📝 Walkthrough

Walkthrough

Adds StringUtil::calcLevenshteinDistance using rolling-row dynamic programming, then wires a new findClosestInScopeName helper into SemanticsExprVisitor::visitVarExpr to attach "did you mean …?" suggestions to UndefinedIdentifier diagnostics. The diagnostic definition is extended with a conditional suggestion note, and several test files are updated to match the new output.

Changes

Did-you-mean suggestion for undefined identifiers

Layer / File(s)	Summary
Levenshtein distance API and unit tests `source/core/slang-string-util.h`, `source/core/slang-string-util.cpp`, `tools/slang-unit-test/unit-test-string-util.cpp`	Declares and implements `calcLevenshteinDistance` and `calcLevenshteinDistanceCaseInsensitive` on `StringUtil` using two rolling DP rows sized to the second input's length; adds `slang-math.h` include for `Math::Min`. Unit tests cover empty, identical, insertion, deletion, substitution, adjacent edits, transposition (counted as two edits), a textbook example, and symmetry.
Scope-aware suggestion lookup and diagnostic wiring `source/slang/slang-check-expr.cpp`, `source/slang/slang-diagnostics.lua`	Adds `findClosestInScopeName`, which walks the lexical scope chain and sibling scopes, excludes core-module candidates, and applies a length-scaled distance threshold using case-insensitive `calcLevenshteinDistance`. `visitVarExpr` calls it on lookup failure and passes the result and its location through `Diagnostics::UndefinedIdentifier`; the diagnostic definition gains an optional "did you mean …?" note keyed on `suggestionLocation`.
New and updated diagnostic test expectations `tests/diagnostics/did-you-mean-identifier.slang`, `tests/diagnostics/autodiff-custom-diff-unresolved.slang`, `tests/diagnostics/transitive-namespace-import.slang/test.slang`	Adds a new test file with eight diagnostic cases: close-match (misspelled identifier within distance threshold), no-match (identifier too distant), too-short identifier (below minimum length threshold), distance-cap boundary (exactly at cap), distance-cap exceed (one past cap), core-module exclusion (typo of builtin type not suggested), tie-break suppression (two equally close candidates), and inherited-member limitation (fields not found in derived scopes). Updates autodiff and transitive-namespace-import CHECK expectations to include new suggestion output.

Suggested reviewers

bmillsNV

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 30.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: adding a 'did you mean' suggestion feature for undefined-identifier errors, with specific issue reference.
Description check	✅ Passed	The description is well-related to the changeset, explaining the feature, implementation approach, design decisions, and issue reference.
Linked Issues check	✅ Passed	The PR fully addresses issue `#9124` requirements: implements Levenshtein distance algorithm, performs scope-aware name lookup, and integrates suggestions with undefined-identifier diagnostics.
Out of Scope Changes check	✅ Passed	All changes are directly related to the 'did you mean' feature: string utility functions, diagnostic integration, scope analysis, and comprehensive test coverage are all within scope.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: be90e543-fe80-46cc-b35a-b525afab9baf

📥 Commits

Reviewing files that changed from the base of the PR and between d620d71 and 26711a7.

📒 Files selected for processing (8)

source/core/slang-string-util.cpp
source/core/slang-string-util.h
source/slang/slang-check-expr.cpp
source/slang/slang-diagnostics.lua
tests/diagnostics/autodiff-custom-diff-unresolved.slang
tests/diagnostics/did-you-mean-identifier.slang
tests/diagnostics/transitive-namespace-import.slang/test.slang
tools/slang-unit-test/unit-test-string-util.cpp

Copilot

Pull request overview

This PR adds “did you mean ‘…’?” suggestions to the existing undefined identifier diagnostic (E30015) by scanning in-scope names, using a conservative Levenshtein-distance heuristic to avoid noisy suggestions, and validating the behavior with new unit + diagnostic tests.

Changes:

Add StringUtil::calcLevenshteinDistance() (rolling-row DP) plus unit tests.
Extend E30015 (undefined-identifier) to optionally emit a note: did you mean '…'?.
Implement scope-walk + nearest-name selection in visitVarExpr() and add regression diagnostics tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tools/slang-unit-test/unit-test-string-util.cpp	Adds unit tests for `StringUtil::calcLevenshteinDistance()`.
tests/diagnostics/transitive-namespace-import.slang/test.slang	Updates expectations to include the new “did you mean …?” note where applicable.
tests/diagnostics/did-you-mean-identifier.slang	New regression test for E30015 suggestion note (plus a “no suggestion” case).
tests/diagnostics/autodiff-custom-diff-unresolved.slang	Updates expectations to include the new suggestion note.
source/slang/slang-diagnostics.lua	Extends E30015 definition with an optional note for suggestions.
source/slang/slang-check-expr.cpp	Implements closest-name search on unresolved `VarExpr` and attaches suggestion fields to E30015.
source/core/slang-string-util.h	Declares `StringUtil::calcLevenshteinDistance()`.
source/core/slang-string-util.cpp	Implements Levenshtein distance (but currently has a compilation issue; see comment).

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 50489565-9f6e-4b1d-afdd-f28b91a9fac0

📥 Commits

Reviewing files that changed from the base of the PR and between 26711a7 and 2580892.

📒 Files selected for processing (8)

source/core/slang-string-util.cpp
source/core/slang-string-util.h
source/slang/slang-check-expr.cpp
source/slang/slang-diagnostics.lua
tests/diagnostics/autodiff-custom-diff-unresolved.slang
tests/diagnostics/did-you-mean-identifier.slang
tests/diagnostics/transitive-namespace-import.slang/test.slang
tools/slang-unit-test/unit-test-string-util.cpp

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8033077f-f62b-4b40-b1e1-a97322416b01

📥 Commits

Reviewing files that changed from the base of the PR and between 909a49a and 06ef209.

📒 Files selected for processing (8)

source/core/slang-string-util.cpp
source/core/slang-string-util.h
source/slang/slang-check-expr.cpp
source/slang/slang-diagnostics.lua
tests/diagnostics/autodiff-custom-diff-unresolved.slang
tests/diagnostics/did-you-mean-identifier.slang
tests/diagnostics/transitive-namespace-import.slang/test.slang
tools/slang-unit-test/unit-test-string-util.cpp

coderabbitai

Actionable comments posted: 1

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 15fcaf5a-fc6d-4a8b-9cbb-bb5d5f9593ca

📥 Commits

Reviewing files that changed from the base of the PR and between 06ef209 and 44349dd.

📒 Files selected for processing (8)

source/core/slang-string-util.cpp
source/core/slang-string-util.h
source/slang/slang-check-expr.cpp
source/slang/slang-diagnostics.lua
tests/diagnostics/autodiff-custom-diff-unresolved.slang
tests/diagnostics/did-you-mean-identifier.slang
tests/diagnostics/transitive-namespace-import.slang/test.slang
tools/slang-unit-test/unit-test-string-util.cpp

…shader-slang#9124)

jkiviluoto-nv

LGTM

Pin that a >256-char typo gets no "did you mean" suggestion (the upper-bound guard against an O(N*M) Levenshtein DP on adversarial identifiers). Uses a FileCheck CHECK-NOT directive rather than DIAGNOSTIC_TEST to avoid 260-column caret annotations.

github-actions

Verdict: 🟡 Has issues — 0 bug(s), 2 gap(s)

PR adds a "did you mean ''?" note to E30015 (undefined identifier) by introducing StringUtil::calcLevenshteinDistance and a findClosestInScopeName helper that walks the lexical scope chain, applies length-scaled distance thresholds (3..256 char target, edit distance ≤ min(3, max(1, len/3))), filters via isFromCoreModule and isDeclVisibleFromScope, and suppresses on tie. The two non-suggesting Diagnostics::UndefinedIdentifier call sites (getTypeForDeclRef, __target_intrinsic scrutinee) are documented in comments. Findings are test-coverage gaps around behaviors the PR explicitly designs for but does not pin under test.

Changes Overview

Levenshtein utility (source/core/slang-string-util.cpp, source/core/slang-string-util.h, tools/slang-unit-test/unit-test-string-util.cpp)

Adds case-sensitive calcLevenshteinDistance and case-insensitive calcLevenshteinDistanceCaseInsensitive over UnownedStringSlice. Shared _calcLevenshteinDistance core with a caseInsensitive flag and a two-rolling-row DP (O(lenB) working set). Unit tests cover identical/empty/insertion/deletion/substitution/kitten→sitting/symmetry/case-folding.

Suggestion at visitVarExpr (source/slang/slang-check-expr.cpp, source/slang/slang-diagnostics.lua)

Adds findClosestInScopeName (static helper) and calls it from visitVarExpr on lookup failure. The lua undefined-identifier diagnostic is extended with an optional note span keyed on a suggestionLocation field; an invalid SourceLoc{} suppresses the note.

Non-suggesting sites documented (source/slang/slang-check-decl.cpp, source/slang/slang-check-modifier.cpp)

Adds comments at the two other Diagnostics::UndefinedIdentifier call sites (getTypeForDeclRef and the __target_intrinsic scrutinee in checkModifier) explaining why no suggestion is attached there.

Tests (tests/diagnostics/did-you-mean-identifier.slang, tests/diagnostics/did-you-mean-identifier-too-long.slang, tests/diagnostics/autodiff-custom-diff-unresolved.slang, tests/diagnostics/transitive-namespace-import.slang/*)

New DIAGNOSTIC_TEST exhaustive-mode test exercises close-match, no-match, min-length on/off, case-insensitive, distance-cap on/over, core-module exclusion, tie-break, same-name overload dedup, and struct-member self lookup. Separate SIMPLE filecheck test exercises the >256 char guard via CHECK-NOT: did you mean. Cross-module test pins suggesting the public f_b and NOT suggesting the non-public f_private from another module.

Findings (2 total)

Severity	Location	Finding
🟡 Gap	`tests/diagnostics/did-you-mean-identifier.slang`	No end-to-end test for the `getDeclToExcludeFromLookup()` self-exclusion path (`int x = x;`-style typo).
🟡 Gap	`source/slang/slang-check-modifier.cpp:1972`	The "no suggestion at `__target_intrinsic`" (and the parallel `getTypeForDeclRef`) decision is documented only in a comment, not pinned by `CHECK-NOT` in a test.

github-actions · 2026-06-25T01:51:04Z

+//CHECK:       ^^^^^^^^^^ undefined identifier 'lengthFiel'.
+//CHECK:       ^^^^^^^^^^ did you mean 'lengthField'?
+    }
+}


🟡 Gap: no regression test for the getDeclToExcludeFromLookup() exclusion

The PR explicitly threads getDeclToExcludeFromLookup() through to the suggestion search (in findClosestInScopeName: if (candidateDecl == declToExclude) continue;) and the function comment says this "mirrors the getDeclToExcludeFromLookup() exclusion that real lookup applies." But none of the new cases here exercises the self-initialization typo case, where the declared variable itself is the closest-named candidate.

Without that test, a future change that drops the declToExclude argument or silently passes nullptr would still produce a diagnostic with a suggestion that names the variable being declared — turning int longIdentifier = longIdentifie; into "did you mean 'longIdentifier'?" — and the suite would not catch it.

Suggestion: add one more case to did-you-mean-identifier.slang:

float selfInit() { float longIdentifier = longIdentifie; //CHECK: ^^^^^^^^^^^^^ undefined identifier //CHECK: ^^^^^^^^^^^^^ undefined identifier 'longIdentifie'. return longIdentifier; }

Exhaustive-mode (DIAGNOSTIC_TEST) means a spurious "did you mean 'longIdentifier'?" would be an unannotated diagnostic and fail the test, pinning the contract.

github-actions · 2026-06-25T01:51:07Z

                if (!scrutineeResults.isValid())
                {
+                    // No "did you mean ...?" suggestion here (unlike the
+                    // `VarExpr` path): a `__target_intrinsic` scrutinee names a


🟡 Gap: the "no suggestion at __target_intrinsic" decision is documented in a comment but not pinned by a test

The PR adds an explanatory comment here (and a similar one in slang-check-decl.cpp:1791) justifying why Diagnostics::UndefinedIdentifier at this site deliberately omits the .suggestion/.suggestionLocation fields. A future contributor refactoring this site (e.g. extracting a shared helper that always populates the suggestion) would silently change behavior, and nothing in the suite would catch it — the comment is the only safeguard.

Suggestion: add a small DIAGNOSTIC_TEST under tests/diagnostics/ that triggers a __target_intrinsic scrutinee with a typo of an in-scope identifier, and use CHECK-NOT: did you mean to pin the intentional absence. Same goes for the contextual-keyword path in slang-check-decl.cpp getTypeForDeclRef.

expipiplus1 requested a review from a team as a code owner June 16, 2026 08:50

expipiplus1 requested review from bmillsNV and Copilot and removed request for a team June 16, 2026 08:50

expipiplus1 added the pr: non-breaking PRs without breaking changes label Jun 16, 2026

Copilot started reviewing on behalf of expipiplus1 June 16, 2026 08:51 View session

coderabbitai Bot reviewed Jun 16, 2026

View reviewed changes

Comment thread source/slang/slang-check-expr.cpp

Copilot AI reviewed Jun 16, 2026

View reviewed changes

Comment thread source/core/slang-string-util.cpp Outdated

expipiplus1 force-pushed the pr/did-you-mean-identifier branch from 26711a7 to 2580892 Compare June 22, 2026 12:59

coderabbitai Bot reviewed Jun 22, 2026

View reviewed changes

Comment thread source/slang/slang-check-expr.cpp

This comment was marked as outdated.

Sign in to view

expipiplus1 force-pushed the pr/did-you-mean-identifier branch from 2580892 to 909a49a Compare June 22, 2026 13:58

This comment was marked as outdated.

Sign in to view

jhelferty-nv assigned expipiplus1 Jun 22, 2026

jhelferty-nv removed the request for review from bmillsNV June 22, 2026 22:13

expipiplus1 force-pushed the pr/did-you-mean-identifier branch from 909a49a to 06ef209 Compare June 23, 2026 01:50

coderabbitai Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread source/slang/slang-check-expr.cpp

This comment was marked as outdated.

Sign in to view

expipiplus1 force-pushed the pr/did-you-mean-identifier branch from 06ef209 to 44349dd Compare June 23, 2026 02:15

coderabbitai Bot reviewed Jun 23, 2026

View reviewed changes

Comment thread source/slang/slang-check-expr.cpp

expipiplus1 force-pushed the pr/did-you-mean-identifier branch from 44349dd to 9c1920b Compare June 23, 2026 02:33

This comment was marked as outdated.

Sign in to view

expipiplus1 force-pushed the pr/did-you-mean-identifier branch from 9c1920b to 224971d Compare June 23, 2026 05:13

This comment was marked as outdated.

Sign in to view

expipiplus1 force-pushed the pr/did-you-mean-identifier branch from 224971d to 6679035 Compare June 23, 2026 06:14

This comment was marked as outdated.

Sign in to view

expipiplus1 enabled auto-merge June 23, 2026 07:37

Suggest a similarly-spelled identifier on undefined-identifier errors (…

fcdb946

…shader-slang#9124)

expipiplus1 force-pushed the pr/did-you-mean-identifier branch from 6679035 to fcdb946 Compare June 23, 2026 07:53

This comment was marked as outdated.

Sign in to view

expipiplus1 disabled auto-merge June 24, 2026 00:39

expipiplus1 enabled auto-merge June 24, 2026 00:39

jkiviluoto-nv previously approved these changes Jun 24, 2026

View reviewed changes

expipiplus1 added this pull request to the merge queue Jun 24, 2026

jkiviluoto-nv removed this pull request from the merge queue due to a manual request Jun 24, 2026

Merge branch 'master' into pr/did-you-mean-identifier

cf1a9fd

jkiviluoto-nv enabled auto-merge June 24, 2026 09:49

This comment was marked as outdated.

Sign in to view

jkiviluoto-nv added this pull request to the merge queue Jun 24, 2026

expipiplus1 added 2 commits June 25, 2026 09:24

Merge branch 'master' into pr/did-you-mean-identifier

4814b7b

expipiplus1 dismissed jkiviluoto-nv’s stale review via 4814b7b June 25, 2026 01:24

github-actions Bot reviewed Jun 25, 2026

View reviewed changes

expipiplus1 removed this pull request from the merge queue due to a manual request Jun 25, 2026

expipiplus1 merged commit 41dfba7 into shader-slang:master Jun 25, 2026
5 checks passed

Uh oh!

Conversation

expipiplus1 commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviews paused

Walkthrough

Changes

Suggested reviewers

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

jkiviluoto-nv left a comment

Choose a reason for hiding this comment

Uh oh!

This comment was marked as outdated.

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

expipiplus1 commented Jun 16, 2026 •

edited

Loading

coderabbitai Bot commented Jun 16, 2026 •

edited

Loading