inclyc added inline comments.
================ 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) { ---------------- tbaeder wrote: > 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`? `LValueBase` seems to be a wrapper of a pointer that has its own dyn_cast method. I have changed code here like this, but it cannot compile ``` auto *MaybeStringLiteral = dyn_cast_or_null<StringLiteral *>(Result.Val.getLValueBase()); if (MaybeStringLiteral) { return checkFormatStringExpr(S, MaybeStringLiteral, Args, APK, format_idx, firstDataArg, Type, CallType, /*InFunctionCall*/ false, CheckedVarArgs, UncoveredArg, Offset); } ``` ``` <workspace>/llvm-project/llvm/include/llvm/Support/Casting.h:64:53: error: type 'clang::StringLiteral *' cannot be used prior to '::' because it has no members static inline bool doit(const From &Val) { return To::classof(&Val); } ``` ================ 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}} ---------------- tbaeder wrote: > inclyc wrote: > > These new notes are FixIt hints, looks much better than before. > Can you show some sample output? Source: ``` // sample.cpp #include <cstdio> int main() { int *i; scanf(1 ? "%s %d" : "%d", i); } ``` previous version: ``` sample.cpp:5:29: warning: format specifies type 'char *' but the argument has type 'int *' [-Wformat] scanf(1 ? "%s %d" : "%d", i); ~~ ^ %d sample.cpp:5:18: warning: more '%' conversions than data arguments [-Wformat-insufficient-args] scanf(1 ? "%s %d" : "%d", i); ~^ 2 warnings generated. ``` this patch highlights ` cond ? T : F` expressions: ``` sample.cpp:5:29: warning: format specifies type 'char *' but the argument has type 'int *' [-Wformat] scanf(1 ? "%s %d" : "%d", i); ~~~~~~~~~~~~~~~~~~ ^ sample.cpp:5:14: note: format string is defined here scanf(1 ? "%s %d" : "%d", i); ^~ %d sample.cpp:5:9: warning: more '%' conversions than data arguments [-Wformat-insufficient-args] scanf(1 ? "%s %d" : "%d", i); ^~~~~~~~~~~~~~~~~~ sample.cpp:5:18: note: format string is defined here scanf(1 ? "%s %d" : "%d", i); ~^ 2 warnings generated. ``` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130906/new/ https://reviews.llvm.org/D130906 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits