NoQ added a comment.
Looks great! Nitpicks.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:872
+ assert(Loc.isValid() && "Expected the source location of the last"
+ "character of an AST Node is alwasy valid");
+ return Loc;
----------------
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:886-887
+ // `Lexer::getLocForEndOfToken`)
+ assert(Loc.isValid() && "Expected the source location just past the last "
+ "character of an AST Node is alwasy valid");
+ return Loc;
----------------
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:950
+ if (Init->isNullPointerConstant(
+ std::remove_const_t<ASTContext &>(Ctx),
+ // FIXME: Why does this function not ask for `const ASTContext
----------------
It's a bit intrusive but probably nicer to simply remove `const` from all our
`ASTContext &` parameters.
================
Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1034-1035
+ assert(!InitFixIts.empty() &&
+ "Expected at least one fix-it for an initializer of an unsafe "
+ "variable declaration.");
// The loc right before the initializer:
----------------
The old check was saying "`populateInitializerFixItWithSpan()` returns `{}` on
error". So the new check is additionally saying
"`populateInitializerFixItWithSpan()` isn't allowed to have errors anymore",
which is arguably the more important message 🤔
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D143680/new/
https://reviews.llvm.org/D143680
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
