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

[YingChi Long via Phabricator via cfe-commits]

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