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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits