Skip to content

[clang] Add source range to 'use of undeclared identifier' diagnostics #117671

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.

Sign up for GitHub

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
Apr 18, 2025
Merged

[clang] Add source range to 'use of undeclared identifier' diagnostics #117671

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c9d6813
[clang] Add source range to 'use of undeclared identifier' diagnostics
tbaederr Nov 26, 2024
b392044
Rewrite check* functions in DiagnosticRenderer.cpp
tbaederr Nov 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 27 additions & 48 deletions clang/lib/Frontend/DiagnosticRenderer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -454,62 +454,41 @@ 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;
SmallVector<CharSourceRange> SpellingRanges;
mapDiagnosticRanges(Loc, Ranges, SpellingRanges);

// Count all valid ranges.
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;
}
Expand Down Expand Up @@ -539,13 +518,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);

Expand Down
45 changes: 24 additions & 21 deletions clang/lib/Sema/SemaExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2376,20 +2376,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;
}

Expand All @@ -2400,12 +2402,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));
}

Expand Down Expand Up @@ -2474,6 +2477,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;
Expand Down Expand Up @@ -2531,13 +2535,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);
Expand Down Expand Up @@ -2616,12 +2619,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.
Expand All @@ -2634,13 +2638,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;
}

Expand Down
Loading