[PATCH] D130906: [clang] format string checking for conpile-time evaluated str literal

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for working on this, this is heading in the right direction! Please be sure to also add a release note so users know about the new functionality.



================
Comment at: clang/lib/Sema/SemaChecking.cpp:8509
+  if (E->EvaluateAsRValue(Result, S.Context) && Result.Val.isLValue()) {
+    auto *LVE = Result.Val.getLValueBase().dyn_cast<const Expr *>();
+    if (LVE && LVE->getStmtClass() == Stmt::StringLiteralClass)
----------------



================
Comment at: clang/lib/Sema/SemaChecking.cpp:8510
+    auto *LVE = Result.Val.getLValueBase().dyn_cast<const Expr *>();
+    if (LVE && LVE->getStmtClass() == Stmt::StringLiteralClass)
+      return checkFormatStringExpr(
----------------



================
Comment at: clang/test/Sema/format-strings.c:625
+  printf(1 ? "yes %d" : "no %d"); // expected-warning{{more '%' conversions than data arguments}} \
+                                  // expected-note {{format string is defined here}}
 
----------------
Most of these new notes like this seems chatty -- perhaps we should silence the note when it's going to be located on the same line as the warning?


================
Comment at: clang/test/Sema/format-strings.c:263
   // expected-note at -1{{treat the string as an argument to avoid this}}
-  printf(s4); // expected-warning{{not a string literal}}
-  // expected-note at -1{{treat the string as an argument to avoid this}}
+  printf(s4);
   printf(s5); // expected-warning{{not a string literal}}
----------------
inclyc wrote:
> Here, s4 is declared as `char * const` and initialized as `const char`, so warning generates at the declaration of  `s4` is sufficient? Since modify characters in string literal causes UB.
> 
> ```
> Initializing 'char *const' with an expression of type 'const char[6]' discards qualifiers
> ```
> 
> Currently the checker in this patch just qualifies `"hello"` as a string literal, and does not report `-Wformat-nonliteral` as before
I think we don't want to lose the warning here -- a `char * const` is not a string literal or a reasonable replacement for one.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D130906/new/

https://reviews.llvm.org/D130906