[PATCH] D130906: [clang] format string checking for conpile-time evaluated str literal
Timm Bäder via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 1 21:19:33 PDT 2022
tbaeder added inline comments.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:8504
return SLCT_UncheckedLiteral;
+ Expr::EvalResult Result;
+ if (E->EvaluateAsRValue(Result, S.Context)) {
----------------
A comment above this line would be helpful. Would also visually separate it from the `return` above, which just confused me.
================
Comment at: clang/lib/Sema/SemaChecking.cpp:8507
+ if (Result.Val.isLValue()) {
+ auto *LVE = Result.Val.getLValueBase().dyn_cast<const Expr *>();
+ if (LVE && LVE->getStmtClass() == Stmt::StringLiteralClass) {
----------------
I think you should be able to unify the two `if` statements.
Can you not `dyn_cast_or_null<StringLiteral>(Result.Val.getLValueBase())` here instead of casting to `Expr*` and checking the `StmtClass`?
================
Comment at: clang/test/Sema/format-strings-scanf.c:235
scanf(0 ? "%s" : "%d", i); // no warning
- scanf(1 ? "%s" : "%d", i); // expected-warning{{format specifies type 'char *'}}
+ scanf(1 ? "%s" : "%d", i); // expected-warning{{format specifies type 'char *'}} \
+ // expected-note{{format string is defined here}}
----------------
inclyc wrote:
> These new notes are FixIt hints, looks much better than before.
Can you show some sample output?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D130906/new/
https://reviews.llvm.org/D130906
More information about the cfe-commits
mailing list