[Clang] suggest headers on undeclared errors #140247
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When a “use of undeclared identifier” error happens for a function listed in StdSymbolMap, emit a note telling the user which header to include. Because nested-name-specifier errors occur before the function name is known, perform this lookup later in isCXXDeclarationSpecifier() after a parse failure. Update ParseTentative.cpp accordingly and add tests to verify the suggestions.
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-modules @llvm/pr-subscribers-clang Author: Jongmyeong Choi (jongmyeong-choi) ChangesWhen a “use of undeclared identifier” error happens for a function listed in StdSymbolMap, emit a note telling the user which header to include. Full diff: https://github.com/llvm/llvm-project/pull/140247.diff 12 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 604bb56d28252..bbc8e56175665 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5984,6 +5984,7 @@ def err_unexpected_typedef : Error<
def err_unexpected_namespace : Error<
"unexpected namespace name %0: expected expression">;
def err_undeclared_var_use : Error<"use of undeclared identifier %0">;
+def note_include_for_declaration : Note<"perhaps `#include %0` is needed?">;
def ext_undeclared_unqual_id_with_dependent_base : ExtWarn<
"use of undeclared identifier %0; "
"unqualified lookup into dependent bases of class template %1 is a Microsoft extension">,
diff --git a/clang/lib/Parse/ParseTentative.cpp b/clang/lib/Parse/ParseTentative.cpp
index fcd76c75c9bfb..3cb46cbc4b665 100644
--- a/clang/lib/Parse/ParseTentative.cpp
+++ b/clang/lib/Parse/ParseTentative.cpp
@@ -14,6 +14,8 @@
#include "clang/Basic/DiagnosticParse.h"
#include "clang/Parse/Parser.h"
#include "clang/Sema/ParsedTemplate.h"
+#include "clang/Tooling/Inclusions/StandardLibrary.h"
+
using namespace clang;
/// isCXXDeclarationStatement - C++-specialized function that disambiguates
@@ -1442,6 +1444,34 @@ Parser::isCXXDeclarationSpecifier(ImplicitTypenameContext AllowImplicitTypename,
// If annotation failed, assume it's a non-type.
// FIXME: If this happens due to an undeclared identifier, treat it as
// ambiguous.
+
+ if (Tok.is(tok::annot_cxxscope)) {
+ CXXScopeSpec SS;
+ Actions.RestoreNestedNameSpecifierAnnotation(
+ Tok.getAnnotationValue(), Tok.getAnnotationRange(), SS);
+ auto Next = NextToken();
+ if (SS.isInvalid() && Next.is(tok::identifier)) {
+ auto SymbolName = Next.getIdentifierInfo()->getName();
+ SourceRange SR = Tok.getAnnotationRange();
+ CharSourceRange CSR = CharSourceRange::getCharRange(
+ SR.getBegin(), SR.getEnd().getLocWithOffset(2));
+ StringRef ScopeText = Lexer::getSourceText(CSR, PP.getSourceManager(),
+ PP.getLangOpts());
+
+ auto header = [](llvm::StringRef symbolName, llvm::StringRef scope) {
+ for (const auto &symbol : clang::tooling::stdlib::Symbol::all()) {
+ if (symbol.name() == symbolName && symbol.scope() == scope)
+ return symbol.header()->name();
+ }
+ return llvm::StringRef();
+ }(SymbolName, ScopeText);
+
+ if (header.size() > 0) {
+ Diag(Next.getLocation(), diag::note_include_for_declaration)
+ << header;
+ }
+ }
+ }
if (Tok.is(tok::identifier))
return TPResult::False;
}
diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index 4b87004e4b8ea..de084d01cc553 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -114,4 +114,5 @@ add_clang_library(clangSema
clangEdit
clangLex
clangSupport
- )
+ clangToolingInclusionsStdlib
+)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d72d97addfac2..9c12eea2083b8 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -58,6 +58,7 @@
#include "clang/Sema/SemaOpenMP.h"
#include "clang/Sema/SemaPseudoObject.h"
#include "clang/Sema/Template.h"
+#include "clang/Tooling/Inclusions/StandardLibrary.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/STLForwardCompat.h"
#include "llvm/ADT/StringExtras.h"
@@ -2647,6 +2648,22 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
// Give up, we can't recover.
Diag(R.getNameLoc(), diagnostic) << Name << NameRange;
+
+ llvm::errs() << "cheese ##1) name's kind : " << Name.getAsString() << "\n";
+ auto header = [](const std::string &symbolName) {
+ for (const auto &symbol : clang::tooling::stdlib::Symbol::all()) {
+ if (symbol.header() && symbol.name() == symbolName)
+ return symbol.header()->name();
+ }
+ return llvm::StringRef();
+ }(Name.getAsString());
+
+ llvm::errs() << "cheese ##2) header : " << header << "\n";
+ bool IsFunctionContext = !Args.empty() || ExplicitTemplateArgs != nullptr;
+ if (diagnostic == diag::err_undeclared_var_use && header.size() > 0 &&
+ IsFunctionContext) {
+ Diag(R.getNameLoc(), diag::note_include_for_declaration) << header;
+ }
return true;
}
diff --git a/clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp b/clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
index ce5eefc6bfdb4..60b66aac9e5ca 100644
--- a/clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
+++ b/clang/test/CXX/dcl.decl/dcl.decomp/p3.cpp
@@ -26,14 +26,18 @@ void no_get_1() {
auto [a0, a1] = A(); // expected-error {{decomposes into 3 elements}}
auto [b0, b1] = B(); // expected-error {{decomposes into 3 elements}}
}
- auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
+ auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} \
+ // expected-note {{perhaps `#include <ranges>` is needed?}} \
+ // expected-note {{in implicit initialization of binding declaration 'a0'}}
}
int get(A);
void no_get_2() {
// FIXME: This diagnostic is not great.
- auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}}
+ auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} \
+ // expected-note {{perhaps `#include <ranges>` is needed?}} \
+ // expected-note {{in implicit initialization of binding declaration 'a0'}}
}
template<int> float &get(A); // expected-note 2 {{no known conversion}}
@@ -172,7 +176,9 @@ template<int> int get(ADL::X);
template<> struct std::tuple_size<ADL::X> { static const int value = 1; };
template<> struct std::tuple_element<0, ADL::X> { typedef int type; };
void adl_only_bad() {
- auto [x] = ADL::X(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit init}}
+ auto [x] = ADL::X(); // expected-error {{undeclared identifier 'get'}} \
+ // expected-note {{perhaps `#include <ranges>` is needed?}} \
+ // expected-note {{in implicit init}}
}
template<typename ElemType, typename GetTypeLV, typename GetTypeRV>
diff --git a/clang/test/CXX/drs/cwg3xx.cpp b/clang/test/CXX/drs/cwg3xx.cpp
index 8b035cf6f2370..c94c478d8e3ac 100644
--- a/clang/test/CXX/drs/cwg3xx.cpp
+++ b/clang/test/CXX/drs/cwg3xx.cpp
@@ -1474,6 +1474,7 @@ namespace cwg387 { // cwg387: 2.8
a = gcd(a, b);
b = gcd(3, 4);
// expected-error@-1 {{use of undeclared identifier 'gcd'}}
+ // expected-note@-2 {{perhaps `#include <numeric>` is needed?}}
}
}
@@ -1489,6 +1490,7 @@ namespace cwg387 { // cwg387: 2.8
a = gcd(a, b);
b = gcd(3, 4);
// expected-error@-1 {{use of undeclared identifier 'gcd'}}
+ // expected-note@-2 {{perhaps `#include <numeric>` is needed?}}
}
}
} // namespace cwg387
diff --git a/clang/test/Modules/implicit-declared-allocation-functions.cppm b/clang/test/Modules/implicit-declared-allocation-functions.cppm
index b378a1d1365ee..5507e19fda83d 100644
--- a/clang/test/Modules/implicit-declared-allocation-functions.cppm
+++ b/clang/test/Modules/implicit-declared-allocation-functions.cppm
@@ -17,14 +17,14 @@ export void alloc_wrapper() {
// std::align_val_t is ill-formed unless a standard library declaration
// ([cstddef.syn], [new.syn], [std.modules]) of that name precedes
// ([basic.lookup.general]) the use of that name.
- void *b = ::operator new((std::size_t)32); // expected-error {{use of undeclared identifier 'std'}}
- void *c = ::operator new((std::size_t)32, // expected-error {{use of undeclared identifier 'std'}}
- (std::align_val_t)64); // expected-error {{use of undeclared identifier 'std'}}
+ void *b = ::operator new((std::size_t)32); // expected-error {{use of undeclared identifier 'std'}} expected-note {{perhaps `#include <cstddef>` is needed?}}
+ void *c = ::operator new((std::size_t)32, // expected-error {{use of undeclared identifier 'std'}} expected-note {{perhaps `#include <cstddef>` is needed?}}
+ (std::align_val_t)64); // expected-error {{use of undeclared identifier 'std'}} expected-note {{perhaps `#include <new>` is needed?}}
::operator delete(a);
- ::operator delete(b, (std::size_t)32); // expected-error {{use of undeclared identifier 'std'}}
- ::operator delete(c, (std::size_t)32, // expected-error {{use of undeclared identifier 'std'}}
- (std::align_val_t)64); // expected-error {{use of undeclared identifier 'std'}}
+ ::operator delete(b, (std::size_t)32); // expected-error {{use of undeclared identifier 'std'}} expected-note {{perhaps `#include <cstddef>` is needed?}}
+ ::operator delete(c, (std::size_t)32, // expected-error {{use of undeclared identifier 'std'}} expected-note {{perhaps `#include <cstddef>` is needed?}}
+ (std::align_val_t)64); // expected-error {{use of undeclared identifier 'std'}} expected-note {{perhaps `#include <new>` is needed?}}
}
//--- new
diff --git a/clang/test/Modules/macro-reexport.cpp b/clang/test/Modules/macro-reexport.cpp
index 4e825a07bd803..145b2e9c5d01b 100644
--- a/clang/test/Modules/macro-reexport.cpp
+++ b/clang/test/Modules/macro-reexport.cpp
@@ -21,13 +21,13 @@
#include "f1.h"
void f() { return assert(true); } // expected-error {{undeclared identifier 'd'}}
#include "e2.h" // undefines d1's macro
-void g() { return assert(true); } // expected-error {{undeclared identifier 'assert'}}
+void g() { return assert(true); } // expected-error {{undeclared identifier 'assert'}} expected-note {{perhaps `#include <cassert>` is needed?}}
#elif defined(D1)
#include "e1.h" // undefines c1's macro but not d1's macro
#include "d1.h"
void f() { return assert(true); } // expected-error {{undeclared identifier 'd'}}
#include "e2.h" // undefines d1's macro
-void g() { return assert(true); } // expected-error {{undeclared identifier 'assert'}}
+void g() { return assert(true); } // expected-error {{undeclared identifier 'assert'}} expected-note {{perhaps `#include <cassert>` is needed?}}
#elif defined(D2)
#include "d2.h"
void f() { return assert(true); } // expected-error {{undeclared identifier 'b'}}
@@ -35,5 +35,5 @@ void f() { return assert(true); } // expected-error {{undeclared identifier 'b'}
// e2 undefines d1's macro, which overrides c1's macro.
#include "e2.h"
#include "c1.h"
-void f() { return assert(true); } // expected-error {{undeclared identifier 'assert'}}
+void f() { return assert(true); } // expected-error {{undeclared identifier 'assert'}} expected-note {{perhaps `#include <cassert>` is needed?}}
#endif
diff --git a/clang/test/OpenMP/allocate_modifiers_messages.cpp b/clang/test/OpenMP/allocate_modifiers_messages.cpp
index 6867e78a89ee9..2e6fa37096973 100644
--- a/clang/test/OpenMP/allocate_modifiers_messages.cpp
+++ b/clang/test/OpenMP/allocate_modifiers_messages.cpp
@@ -108,6 +108,7 @@ int main() {
#pragma omp scope private(b) allocate(align
// expected-error@+1 {{duplicate modifier 'align' in 'allocate' clause}}
#pragma omp scope private(a) allocate(align(8), align(4) : a)
+ // expected-note@+6 {{perhaps `#include <memory>` is needed?}}
// expected-error@+5 {{use of undeclared identifier 'align'}}
// expected-error@+4 {{expected ',' or ')' in 'allocate' clause}}
// expected-error@+3 {{expected ')'}}
diff --git a/clang/test/Sema/builtin-setjmp.c b/clang/test/Sema/builtin-setjmp.c
index a71f87162612d..3369468a4a79d 100644
--- a/clang/test/Sema/builtin-setjmp.c
+++ b/clang/test/Sema/builtin-setjmp.c
@@ -35,11 +35,13 @@ void use(void) {
setjmp(0);
#if NO_SETJMP
// cxx-error@-2 {{undeclared identifier 'setjmp'}}
- // c-error@-3 {{call to undeclared function 'setjmp'; ISO C99 and later do not support implicit function declarations}}
+ // cxx-note@-3 {{perhaps `#include <csetjmp>` is needed?}}
+ // c-error@-4 {{call to undeclared function 'setjmp'; ISO C99 and later do not support implicit function declarations}}
#elif ONLY_JMP_BUF
- // cxx-error@-5 {{undeclared identifier 'setjmp'}}
- // c-error@-6 {{call to undeclared library function 'setjmp' with type 'int (jmp_buf)' (aka 'int (int *)'); ISO C99 and later do not support implicit function declarations}}
- // c-note@-7 {{include the header <setjmp.h> or explicitly provide a declaration for 'setjmp'}}
+ // cxx-error@-6 {{undeclared identifier 'setjmp'}}
+ // cxx-note@-7 {{perhaps `#include <csetjmp>` is needed?}}
+ // c-error@-8 {{call to undeclared library function 'setjmp' with type 'int (jmp_buf)' (aka 'int (int *)'); ISO C99 and later do not support implicit function declarations}}
+ // c-note@-9 {{include the header <setjmp.h> or explicitly provide a declaration for 'setjmp'}}
#else
// cxx-no-diagnostics
#endif
diff --git a/clang/test/Sema/note-suggest-header.cpp b/clang/test/Sema/note-suggest-header.cpp
new file mode 100644
index 0000000000000..7444763224903
--- /dev/null
+++ b/clang/test/Sema/note-suggest-header.cpp
@@ -0,0 +1,10 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+void f(void) {
+ int_val2 = 0; // expected-error{{use of undeclared identifier}}
+ sin(0); // expected-error{{use of undeclared identifier 'sin'}} \
+ // expected-note{{perhaps `#include <cmath>` is needed?}}
+
+ std::cout << "Hello world\n"; // expected-error{{use of undeclared identifier 'std'}} \
+ // expected-note{{perhaps `#include <iostream>` is needed?}}
+}
\ No newline at end of file
diff --git a/clang/test/SemaCXX/no-implicit-builtin-decls.cpp b/clang/test/SemaCXX/no-implicit-builtin-decls.cpp
index d82f7f18bffe0..d848c9740dc6e 100644
--- a/clang/test/SemaCXX/no-implicit-builtin-decls.cpp
+++ b/clang/test/SemaCXX/no-implicit-builtin-decls.cpp
@@ -1,7 +1,7 @@
// RUN: %clang_cc1 -fsyntax-only -verify %s
void f() {
- void *p = malloc(sizeof(int) * 10); // expected-error{{use of undeclared identifier 'malloc'}}
+ void *p = malloc(sizeof(int) * 10); // expected-error{{use of undeclared identifier 'malloc'}} expected-note {{perhaps `#include <cstdlib>` is needed?}}
}
int malloc(double);
|
Endilll
reviewed
May 16, 2025
|
|
||
| std::cout << "Hello world\n"; // expected-error{{use of undeclared identifier 'std'}} \ | ||
| // expected-note{{perhaps `#include <iostream>` is needed?}} | ||
| } |
There was a problem hiding this comment.
Add an empty newline at the end
| @@ -1489,6 +1490,7 @@ namespace cwg387 { // cwg387: 2.8 | |||
| a = gcd(a, b); | |||
| b = gcd(3, 4); | |||
| // expected-error@-1 {{use of undeclared identifier 'gcd'}} | |||
| // expected-note@-2 {{perhaps `#include <numeric>` is needed?}} | |||
There was a problem hiding this comment.
Suggested change
| // expected-note@-2 {{perhaps `#include <numeric>` is needed?}} | |
| // expected-note@-2 {{perhaps `#include <numeric>` is needed?}} |
| @@ -1474,6 +1474,7 @@ namespace cwg387 { // cwg387: 2.8 | |||
| a = gcd(a, b); | |||
| b = gcd(3, 4); | |||
| // expected-error@-1 {{use of undeclared identifier 'gcd'}} | |||
| // expected-note@-2 {{perhaps `#include <numeric>` is needed?}} | |||
There was a problem hiding this comment.
Suggested change
| // expected-note@-2 {{perhaps `#include <numeric>` is needed?}} | |
| // expected-note@-2 {{perhaps `#include <numeric>` is needed?}} |
| // expected-note{{perhaps `#include <cmath>` is needed?}} | ||
|
|
||
| std::cout << "Hello world\n"; // expected-error{{use of undeclared identifier 'std'}} \ | ||
| // expected-note{{perhaps `#include <iostream>` is needed?}} |
There was a problem hiding this comment.
This note sounds like <iostream> provides std, which doesn't feel right.
| @@ -26,14 +26,18 @@ void no_get_1() { | |||
| auto [a0, a1] = A(); // expected-error {{decomposes into 3 elements}} | |||
| auto [b0, b1] = B(); // expected-error {{decomposes into 3 elements}} | |||
| } | |||
| auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} expected-note {{in implicit initialization of binding declaration 'a0'}} | |||
| auto [a0, a1, a2] = A(); // expected-error {{undeclared identifier 'get'}} \ | |||
| // expected-note {{perhaps `#include <ranges>` is needed?}} \ | |||
There was a problem hiding this comment.
<ranges> is far from the only header that provides overloads of std::get, but probably the heaviest one for compile times. Also, this code is run in C++17 mode, and there's no <ranges> header in C++17.
There was a problem hiding this comment.
Yeah, this does not seem helpful, it feels actively harmful actually.
There was a problem hiding this comment.
Oh dear. StdSpecialSymbolMap doesn't even specify the namespace corresponding to get(). Thanks. I didn't know there was such a point.
|
You should put "Fixes #120388" in the description, and remove the issue number from the title, because it will be confusing after merging, when number of this PR will be added to the title. |
jongmyeong-choi
changed the title
|
I can only review this on a high level, so CC @AaronBallman to check if this is happening at the right level. |
|
In the case of get(), since the namespace is not specified, it is difficult to find it in the symbol map. I am rewriting the change in the direction of making suggestions only for cases where namespaces are used, such as std::cout. I think we should also consider utilizing codes like the header cleaner of the clang tool, but the job size seems quite large if we do that. |
| @@ -14,6 +14,8 @@ | |||
| #include "clang/Basic/DiagnosticParse.h" | |||
| #include "clang/Parse/Parser.h" | |||
| #include "clang/Sema/ParsedTemplate.h" | |||
| #include "clang/Tooling/Inclusions/StandardLibrary.h" | |||
There was a problem hiding this comment.
This seems like a layering violation, though not a circular dependency. Parser currently does not link against tooling. In your patch, you link tooling into Sema which Parser then picks up transitively. Tooling does not currently link in Parser or Sema, hence no circular dependency.
I kind of think the standard library stuff needs to be lifted into AST. It does seem to rely on Decl.h internally, so it can't be lifted into Basic.
CC @erichkeane for other opinions
There was a problem hiding this comment.
Sure, but we want to rewrite anyway. The implementation (StandardLibrary.cpp) seems fairly inefficient. We should try to TableGen a trie and store version information in there, have some support for typo correction, etc
|
We already do have some typo correction https://compiler-explorer.com/z/Ede9rzsn5
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When a “use of undeclared identifier” error happens for a function listed in StdSymbolMap, emit a note telling the user which header to include.
Because nested-name-specifier errors occur before the function name is known, perform this lookup later in isCXXDeclarationSpecifier() after a parse failure.