Skip to content

[Clang] Don't ditch typo-corrected lookup result #139374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 10, 2025
Merged

Conversation

zyn0217
Copy link
Contributor

@zyn0217 zyn0217 commented May 10, 2025

For a member function call like 'foo.bar()', there are two typo-correction points after parsing the dot. The first occurs in ParseOptionalCXXScopeSpecifier, which tries to annotate the template name following any scope specifiers.

If the template name bar is not found within 'foo', the parser was previously instructed to drop any function templates found outside of the scope. This was intended to prevent ambiguity in expressions like 'foo->bar < 7', as explained in commit 50a3cdd. However, it's unnecessary to discard typo-corrected results that were strictly resolved within the scope 'foo'.

We won't perform a second typo-correction in ParseUnqualifiedId after the name being annotated.

Fixes #139226

For a member function call like 'foo.bar<int>()', there are two
typo-correction points after parsing the dot. The first occurs in
ParseOptionalCXXScopeSpecifier, which tries to annotate the template
name following any scope specifiers.

If the template name bar is not found within 'foo', the parser was
previously instructed to drop any function templates found outside of
the scope. This was intended to prevent ambiguity in expressions
like 'foo->bar < 7', as explained in commit 50a3cdd. However, it's
unnecessary to discard typo-corrected results that were strictly
resolved within the scope 'foo'.

We won't perform a second typo-correction in ParseUnqualifiedId after
the name being annotated.
@zyn0217 zyn0217 requested review from cor3ntin and erichkeane May 10, 2025 11:17
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels May 10, 2025
@llvmbot
Copy link
Member

llvmbot commented May 10, 2025

@llvm/pr-subscribers-clang

Author: Younan Zhang (zyn0217)

Changes

For a member function call like 'foo.bar<int>()', there are two typo-correction points after parsing the dot. The first occurs in ParseOptionalCXXScopeSpecifier, which tries to annotate the template name following any scope specifiers.

If the template name bar is not found within 'foo', the parser was previously instructed to drop any function templates found outside of the scope. This was intended to prevent ambiguity in expressions like 'foo->bar < 7', as explained in commit 50a3cdd. However, it's unnecessary to discard typo-corrected results that were strictly resolved within the scope 'foo'.

We won't perform a second typo-correction in ParseUnqualifiedId after the name being annotated.

Fixes #139226


Full diff: https://github.com/llvm/llvm-project/pull/139374.diff

3 Files Affected:

  • (modified) clang/docs/ReleaseNotes.rst (+4-1)
  • (modified) clang/lib/Sema/SemaTemplate.cpp (+3)
  • (modified) clang/test/CXX/basic/basic.lookup/basic.lookup.classref/p1.cpp (+13)
diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 1d0896f585fb4..234c28a143cf1 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -517,7 +517,10 @@ Improvements to Clang's diagnostics
 
 - Improved the ``-Wtautological-overlap-compare`` diagnostics to warn about overlapping and non-overlapping ranges involving character literals and floating-point literals. 
   The warning message for non-overlapping cases has also been improved (#GH13473).
-  
+
+- Fixed a duplicate diagnostic when performing typo correction on function template
+  calls with explicit template arguments. Fixes #GH139226.
+
 Improvements to Clang's time-trace
 ----------------------------------
 
diff --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 2ba69c87d95e3..a18b545a5feea 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -523,6 +523,9 @@ bool Sema::LookupTemplateName(LookupResult &Found, Scope *S, CXXScopeSpec &SS,
       if (Found.isAmbiguous()) {
         Found.clear();
       } else if (!Found.empty()) {
+        // Do not erase the typo-corrected result to avoid duplicating the typo
+        // correction in future.
+        AllowFunctionTemplatesInLookup = true;
         Found.setLookupName(Corrected.getCorrection());
         if (LookupCtx) {
           std::string CorrectedStr(Corrected.getAsString(getLangOpts()));
diff --git a/clang/test/CXX/basic/basic.lookup/basic.lookup.classref/p1.cpp b/clang/test/CXX/basic/basic.lookup/basic.lookup.classref/p1.cpp
index e3599db18350b..3633ef1c293e2 100644
--- a/clang/test/CXX/basic/basic.lookup/basic.lookup.classref/p1.cpp
+++ b/clang/test/CXX/basic/basic.lookup/basic.lookup.classref/p1.cpp
@@ -98,3 +98,16 @@ namespace PR11856 {
     }
   }
 }
+
+namespace GH139226 {
+
+struct Foo {
+  template <class T> void LookupWithID(); // expected-note {{declared here}}
+};
+
+void test(Foo &f) {
+  f.LookupWithId<int>();
+  // expected-error@-1 {{did you mean 'LookupWithID'}}
+}
+
+}

Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks

@zyn0217 zyn0217 merged commit 802d8d9 into llvm:main May 10, 2025
12 checks passed
@zyn0217 zyn0217 deleted the GH139226 branch May 10, 2025 12:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Duplicate lookup errors
3 participants