Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: In-Reply-To: <llvm.org/llvm/llvm-project/pull/117...@github.com>
https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/117671 >From a55f27b1c479c91807254bd3f62a1771323d9fe1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@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 4ffce2c1236610..65e357c56cfe68 100644 --- a/clang/lib/Sema/SemaExpr.cpp +++ b/clang/lib/Sema/SemaExpr.cpp @@ -2357,20 +2357,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; } @@ -2381,12 +2383,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)); } @@ -2455,6 +2458,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; @@ -2512,13 +2516,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); @@ -2597,12 +2600,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. @@ -2615,13 +2619,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 90fe6760f73d2b3aec75fb54fd4a003889228862 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbae...@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/Frontend/DiagnosticRenderer.cpp | 75 ++++++++--------------- 1 file changed, 27 insertions(+), 48 deletions(-) diff --git a/clang/lib/Frontend/DiagnosticRenderer.cpp b/clang/lib/Frontend/DiagnosticRenderer.cpp index 8b32af13372579..b11806637efda8 100644 --- a/clang/lib/Frontend/DiagnosticRenderer.cpp +++ b/clang/lib/Frontend/DiagnosticRenderer.cpp @@ -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; } @@ -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); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
