Index: clang/lib/Analysis/UnsafeBufferUsage.cpp =================================================================== --- clang/lib/Analysis/UnsafeBufferUsage.cpp +++ clang/lib/Analysis/UnsafeBufferUsage.cpp @@ -1177,42 +1177,40 @@ // Return the source location of the last character of the AST `Node`. template -static SourceLocation getEndCharLoc(const NodeTy *Node, const SourceManager &SM, +static std::optional getEndCharLoc(const NodeTy *Node, const SourceManager &SM, const LangOptions &LangOpts) { unsigned TkLen = Lexer::MeasureTokenLength(Node->getEndLoc(), SM, LangOpts); SourceLocation Loc = Node->getEndLoc().getLocWithOffset(TkLen - 1); - - // We expect `Loc` to be valid. The location is obtained by moving forward - // from the beginning of the token 'len(token)-1' characters. The file ID of - // the locations within a token must be consistent. - assert(Loc.isValid() && "Expected the source location of the last" - "character of an AST Node is alwasy valid"); - return Loc; + if(Loc.isValid()) + return Loc; + else + return {}; } // Return the source location just past the last character of the AST `Node`. template -static SourceLocation getPastLoc(const NodeTy *Node, const SourceManager &SM, +static std::optional getPastLoc(const NodeTy *Node, const SourceManager &SM, const LangOptions &LangOpts) { SourceLocation Loc = Lexer::getLocForEndOfToken(Node->getEndLoc(), 0, SM, LangOpts); - // We expect `Loc` to be valid as it is either associated to a file ID or - // it must be the end of a macro expansion. (see - // `Lexer::getLocForEndOfToken`) - assert(Loc.isValid() && "Expected the source location just past the last " - "character of an AST Node is alwasy valid"); - return Loc; + if(Loc.isValid()) + return Loc; + else + return {}; } // Return text representation of an `Expr`. static StringRef getExprText(const Expr *E, const SourceManager &SM, const LangOptions &LangOpts) { - SourceLocation LastCharLoc = getPastLoc(E, SM, LangOpts); - - return Lexer::getSourceText( - CharSourceRange::getCharRange(E->getBeginLoc(), LastCharLoc), SM, - LangOpts); + std::optional LastCharLoc = getPastLoc(E, SM, LangOpts); + + if(LastCharLoc) + return Lexer::getSourceText( + CharSourceRange::getCharRange(E->getBeginLoc(), LastCharLoc.value()), SM, + LangOpts); + else + return ""; } std::optional @@ -1311,11 +1309,12 @@ // To transform UPC(++p) to UPC((p = p.subspan(1)).data()): SS << "(" << varName.data() << " = " << varName.data() << ".subspan(1)).data()"; - Fixes.push_back(FixItHint::CreateReplacement( - SourceRange(PreIncNode->getBeginLoc(), - getEndCharLoc(PreIncNode, Ctx.getSourceManager(), - Ctx.getLangOpts())), - SS.str())); + std::optional location = + getEndCharLoc(PreIncNode, Ctx.getSourceManager(), Ctx.getLangOpts()); + + if(location) + Fixes.push_back(FixItHint::CreateReplacement( + SourceRange(PreIncNode->getBeginLoc(), location.value()), SS.str())); return Fixes; } } @@ -1350,9 +1349,13 @@ // &`? It should. Maybe worth an NFC patch later. Expr::NullPointerConstantValueDependence:: NPC_ValueDependentIsNotNull)) { - SourceRange SR(Init->getBeginLoc(), getEndCharLoc(Init, SM, LangOpts)); - - return {FixItHint::CreateRemoval(SR)}; + std::optional location = getEndCharLoc(Init, SM, LangOpts); + if(location) { + SourceRange SR(Init->getBeginLoc(), location.value()); + return {FixItHint::CreateRemoval(SR)}; + } else { + return {}; + } } FixItList FixIts{}; @@ -1392,12 +1395,14 @@ } SmallString<32> StrBuffer{}; - SourceLocation LocPassInit = getPastLoc(Init, SM, LangOpts); + std::optional LocPassInit = getPastLoc(Init, SM, LangOpts); - StrBuffer.append(", "); - StrBuffer.append(ExtentText); - StrBuffer.append("}"); - FixIts.push_back(FixItHint::CreateInsertion(LocPassInit, StrBuffer.str())); + if(LocPassInit) { + StrBuffer.append(", "); + StrBuffer.append(ExtentText); + StrBuffer.append("}"); + FixIts.push_back(FixItHint::CreateInsertion(LocPassInit.value(), StrBuffer.str())); + } return FixIts; } @@ -1420,7 +1425,7 @@ assert(!SpanEltT.isNull() && "Trying to fix a non-pointer type variable!"); FixItList FixIts{}; - SourceLocation + std::optional ReplacementLastLoc; // the inclusive end location of the replacement const SourceManager &SM = Ctx.getSourceManager(); @@ -1443,8 +1448,9 @@ raw_svector_ostream OS(Replacement); OS << "std::span<" << SpanEltT.getAsString() << "> " << D->getName(); - FixIts.push_back(FixItHint::CreateReplacement( - SourceRange{D->getBeginLoc(), ReplacementLastLoc}, OS.str())); + if(ReplacementLastLoc) + FixIts.push_back(FixItHint::CreateReplacement( + SourceRange{D->getBeginLoc(), ReplacementLastLoc.value()}, OS.str())); return FixIts; } Index: clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp =================================================================== --- clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp +++ clang/test/SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp @@ -3,6 +3,8 @@ typedef int * Int_ptr_t; typedef int Int_t; +#define DEFINE_PTR(X) int* ptr = (X); + void local_array_subscript_simple() { int tmp; int *p = new int[10]; @@ -223,3 +225,28 @@ tmp = r[j] + r[k]; // both `j` and `k` are unsigned so they must be non-negative tmp = r[(unsigned int)-1]; // a cast-to-unsigned-expression is also non-negative } + +void all_vars_in_macro() { + int* local; + DEFINE_PTR(local) + ptr[1] = 0; +} + +void few_vars_in_macro() { + int* local; + DEFINE_PTR(local) + ptr[1] = 0; + int tmp; + ptr[2] = 30; + auto p = new int[10]; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:3-[[@LINE-1]]:11}:"std::span p" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:12-[[@LINE-2]]:12}:"{" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-3]]:23-[[@LINE-3]]:23}:", 10}" + tmp = p[5]; + int val = *p; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:13-[[@LINE-1]]:14}:"" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:15-[[@LINE-2]]:15}:"[0]" + val = *p + 30; + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-1]]:9-[[@LINE-1]]:10}:"" + // CHECK-DAG: fix-it:"{{.*}}":{[[@LINE-2]]:11-[[@LINE-2]]:11}:"[0]" +}