[clang] [clang] Add source range to 'use of undeclared identifier' diagnostics (PR #117671)
Timm Baeder via cfe-commits
cfe-commits at lists.llvm.org
Fri Nov 29 06:39:40 PST 2024
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm.org/llvm/llvm-project/pull/117671 at github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/117671
>From 1d77f36a1f7fa974d50ff7a5a98b93bbb01ba26c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Tue, 26 Nov 2024 08:44:57 +0100
Subject: [PATCH 1/2] [clang] Add source range to 'use of undeclared
identifier' diagnostics
---
clang/lib/Sema/SemaExpr.cpp | 45 ++++++++++++++++++++-----------------
1 file changed, 24 insertions(+), 21 deletions(-)
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 6c7472ce92703b..d0d6d981827270 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -2351,20 +2351,22 @@ Sema::DecomposeUnqualifiedId(const UnqualifiedId &Id,
}
}
-static void emitEmptyLookupTypoDiagnostic(
- const TypoCorrection &TC, Sema &SemaRef, const CXXScopeSpec &SS,
- DeclarationName Typo, SourceLocation TypoLoc, ArrayRef<Expr *> Args,
- unsigned DiagnosticID, unsigned DiagnosticSuggestID) {
+static void emitEmptyLookupTypoDiagnostic(const TypoCorrection &TC,
+ Sema &SemaRef, const CXXScopeSpec &SS,
+ DeclarationName Typo,
+ SourceRange TypoRange,
+ unsigned DiagnosticID,
+ unsigned DiagnosticSuggestID) {
DeclContext *Ctx =
SS.isEmpty() ? nullptr : SemaRef.computeDeclContext(SS, false);
if (!TC) {
// Emit a special diagnostic for failed member lookups.
// FIXME: computing the declaration context might fail here (?)
if (Ctx)
- SemaRef.Diag(TypoLoc, diag::err_no_member) << Typo << Ctx
- << SS.getRange();
+ SemaRef.Diag(TypoRange.getBegin(), diag::err_no_member)
+ << Typo << Ctx << TypoRange;
else
- SemaRef.Diag(TypoLoc, DiagnosticID) << Typo;
+ SemaRef.Diag(TypoRange.getBegin(), DiagnosticID) << Typo << TypoRange;
return;
}
@@ -2375,12 +2377,13 @@ static void emitEmptyLookupTypoDiagnostic(
? diag::note_implicit_param_decl
: diag::note_previous_decl;
if (!Ctx)
- SemaRef.diagnoseTypo(TC, SemaRef.PDiag(DiagnosticSuggestID) << Typo,
- SemaRef.PDiag(NoteID));
+ SemaRef.diagnoseTypo(
+ TC, SemaRef.PDiag(DiagnosticSuggestID) << Typo << TypoRange,
+ SemaRef.PDiag(NoteID));
else
- SemaRef.diagnoseTypo(TC, SemaRef.PDiag(diag::err_no_member_suggest)
- << Typo << Ctx << DroppedSpecifier
- << SS.getRange(),
+ SemaRef.diagnoseTypo(TC,
+ SemaRef.PDiag(diag::err_no_member_suggest)
+ << Typo << Ctx << DroppedSpecifier << TypoRange,
SemaRef.PDiag(NoteID));
}
@@ -2449,6 +2452,7 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
ArrayRef<Expr *> Args, DeclContext *LookupCtx,
TypoExpr **Out) {
DeclarationName Name = R.getLookupName();
+ SourceRange NameRange = R.getLookupNameInfo().getSourceRange();
unsigned diagnostic = diag::err_undeclared_var_use;
unsigned diagnostic_suggest = diag::err_undeclared_var_use_suggest;
@@ -2506,13 +2510,12 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
// We didn't find anything, so try to correct for a typo.
TypoCorrection Corrected;
if (S && Out) {
- SourceLocation TypoLoc = R.getNameLoc();
assert(!ExplicitTemplateArgs &&
"Diagnosing an empty lookup with explicit template args!");
*Out = CorrectTypoDelayed(
R.getLookupNameInfo(), R.getLookupKind(), S, &SS, CCC,
[=](const TypoCorrection &TC) {
- emitEmptyLookupTypoDiagnostic(TC, *this, SS, Name, TypoLoc, Args,
+ emitEmptyLookupTypoDiagnostic(TC, *this, SS, Name, NameRange,
diagnostic, diagnostic_suggest);
},
nullptr, CTK_ErrorRecovery, LookupCtx);
@@ -2591,12 +2594,13 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
? diag::note_implicit_param_decl
: diag::note_previous_decl;
if (SS.isEmpty())
- diagnoseTypo(Corrected, PDiag(diagnostic_suggest) << Name,
+ diagnoseTypo(Corrected, PDiag(diagnostic_suggest) << Name << NameRange,
PDiag(NoteID), AcceptableWithRecovery);
else
- diagnoseTypo(Corrected, PDiag(diag::err_no_member_suggest)
- << Name << computeDeclContext(SS, false)
- << DroppedSpecifier << SS.getRange(),
+ diagnoseTypo(Corrected,
+ PDiag(diag::err_no_member_suggest)
+ << Name << computeDeclContext(SS, false)
+ << DroppedSpecifier << NameRange,
PDiag(NoteID), AcceptableWithRecovery);
// Tell the callee whether to try to recover.
@@ -2609,13 +2613,12 @@ bool Sema::DiagnoseEmptyLookup(Scope *S, CXXScopeSpec &SS, LookupResult &R,
// FIXME: computing the declaration context might fail here (?)
if (!SS.isEmpty()) {
Diag(R.getNameLoc(), diag::err_no_member)
- << Name << computeDeclContext(SS, false)
- << SS.getRange();
+ << Name << computeDeclContext(SS, false) << NameRange;
return true;
}
// Give up, we can't recover.
- Diag(R.getNameLoc(), diagnostic) << Name;
+ Diag(R.getNameLoc(), diagnostic) << Name << NameRange;
return true;
}
>From 9379647e15a49eedc3545f2aadaf41e15c8a3545 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Fri, 29 Nov 2024 15:38:24 +0100
Subject: [PATCH 2/2] Rewrite check* functions in DiagnosticRenderer.cpp
In rangeInsideSameMacroArgExpansion, check the given Ranges instead
of the SpellingRanges.
---
clang/lib/Basic/SourceManager.cpp | 9 ++-
clang/lib/Frontend/DiagnosticRenderer.cpp | 87 +++++++++--------------
2 files changed, 40 insertions(+), 56 deletions(-)
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index 6e588ce63d813f..6a9c33395c89c2 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -996,11 +996,13 @@ CharSourceRange SourceManager::getExpansionRange(SourceLocation Loc) const {
bool SourceManager::isMacroArgExpansion(SourceLocation Loc,
SourceLocation *StartLoc) const {
- if (!Loc.isMacroID()) return false;
+ if (!Loc.isMacroID())
+ return false;
FileID FID = getFileID(Loc);
const SrcMgr::ExpansionInfo &Expansion = getSLocEntry(FID).getExpansion();
- if (!Expansion.isMacroArgExpansion()) return false;
+ if (!Expansion.isMacroArgExpansion())
+ return false;
if (StartLoc)
*StartLoc = Expansion.getExpansionLocStart();
@@ -1008,7 +1010,8 @@ bool SourceManager::isMacroArgExpansion(SourceLocation Loc,
}
bool SourceManager::isMacroBodyExpansion(SourceLocation Loc) const {
- if (!Loc.isMacroID()) return false;
+ if (!Loc.isMacroID())
+ return false;
FileID FID = getFileID(Loc);
const SrcMgr::ExpansionInfo &Expansion = getSLocEntry(FID).getExpansion();
diff --git a/clang/lib/Frontend/DiagnosticRenderer.cpp b/clang/lib/Frontend/DiagnosticRenderer.cpp
index 8b32af13372579..78e13ad25c81a4 100644
--- a/clang/lib/Frontend/DiagnosticRenderer.cpp
+++ b/clang/lib/Frontend/DiagnosticRenderer.cpp
@@ -454,62 +454,43 @@ void DiagnosticRenderer::emitSingleMacroExpansion(
SpellingRanges, {});
}
-/// Check that the macro argument location of Loc starts with ArgumentLoc.
-/// The starting location of the macro expansions is used to differeniate
-/// different macro expansions.
-static bool checkLocForMacroArgExpansion(SourceLocation Loc,
- const SourceManager &SM,
- SourceLocation ArgumentLoc) {
- SourceLocation MacroLoc;
- if (SM.isMacroArgExpansion(Loc, &MacroLoc)) {
- if (ArgumentLoc == MacroLoc) return true;
- }
-
- return false;
-}
-
-/// Check if all the locations in the range have the same macro argument
-/// expansion, and that the expansion starts with ArgumentLoc.
-static bool checkRangeForMacroArgExpansion(CharSourceRange Range,
- const SourceManager &SM,
- SourceLocation ArgumentLoc) {
- SourceLocation BegLoc = Range.getBegin(), EndLoc = Range.getEnd();
- while (BegLoc != EndLoc) {
- if (!checkLocForMacroArgExpansion(BegLoc, SM, ArgumentLoc))
- return false;
- BegLoc.getLocWithOffset(1);
- }
-
- return checkLocForMacroArgExpansion(BegLoc, SM, ArgumentLoc);
-}
-
/// A helper function to check if the current ranges are all inside the same
/// macro argument expansion as Loc.
-static bool checkRangesForMacroArgExpansion(FullSourceLoc Loc,
- ArrayRef<CharSourceRange> Ranges) {
+static bool
+rangesInsideSameMacroArgExpansion(FullSourceLoc Loc,
+ ArrayRef<CharSourceRange> Ranges) {
assert(Loc.isMacroID() && "Must be a macro expansion!");
- SmallVector<CharSourceRange, 4> SpellingRanges;
- mapDiagnosticRanges(Loc, Ranges, SpellingRanges);
-
- // Count all valid ranges.
- unsigned ValidCount =
- llvm::count_if(Ranges, [](const auto &R) { return R.isValid(); });
+ {
+ SmallVector<CharSourceRange> SpellingRanges;
+ mapDiagnosticRanges(Loc, Ranges, SpellingRanges);
- if (ValidCount > SpellingRanges.size())
- return false;
+ unsigned ValidCount =
+ llvm::count_if(Ranges, [](const auto &R) { return R.isValid(); });
+ if (ValidCount > SpellingRanges.size())
+ return false;
+ }
- // To store the source location of the argument location.
- FullSourceLoc ArgumentLoc;
+ const SourceManager &SM = Loc.getManager();
+ for (const auto &R : Ranges) {
+ // All positions in the range need to point to Loc.
+ SourceLocation Begin = R.getBegin();
+ if (Begin == R.getEnd()) {
+ if (!SM.isMacroArgExpansion(Begin))
+ return false;
+ continue;
+ }
- // Set the ArgumentLoc to the beginning location of the expansion of Loc
- // so to check if the ranges expands to the same beginning location.
- if (!Loc.isMacroArgExpansion(&ArgumentLoc))
- return false;
+ while (Begin != R.getEnd()) {
+ SourceLocation MacroLoc;
+ if (!SM.isMacroArgExpansion(Begin, &MacroLoc))
+ return false;
+ if (MacroLoc != Loc)
+ return false;
- for (const auto &Range : SpellingRanges)
- if (!checkRangeForMacroArgExpansion(Range, Loc.getManager(), ArgumentLoc))
- return false;
+ Begin = Begin.getLocWithOffset(1);
+ }
+ }
return true;
}
@@ -539,13 +520,13 @@ void DiagnosticRenderer::emitMacroExpansions(FullSourceLoc Loc,
while (L.isMacroID()) {
// If this is the expansion of a macro argument, point the caret at the
// use of the argument in the definition of the macro, not the expansion.
- if (SM.isMacroArgExpansion(L))
+ if (SM.isMacroArgExpansion(L)) {
LocationStack.push_back(SM.getImmediateExpansionRange(L).getBegin());
- else
- LocationStack.push_back(L);
- if (checkRangesForMacroArgExpansion(FullSourceLoc(L, SM), Ranges))
- IgnoredEnd = LocationStack.size();
+ if (rangesInsideSameMacroArgExpansion(FullSourceLoc(L, SM), Ranges))
+ IgnoredEnd = LocationStack.size();
+ } else
+ LocationStack.push_back(L);
L = SM.getImmediateMacroCallerLoc(L);
More information about the cfe-commits
mailing list