[PATCH] D23820: Do not warn about format strings that are indexed string literals.

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/Sema/SemaChecking.cpp:4089-4090
@@ +4088,4 @@
+    if (BinOp->isAdditiveOp()) {
+      bool LIsInt = BinOp->getLHS()->EvaluateAsInt(LResult, S.Context);
+      bool RIsInt = BinOp->getRHS()->EvaluateAsInt(RResult, S.Context);
+
----------------
meikeb wrote:
> rsmith wrote:
> > What happens if one of these expressions is value-dependent? The evaluator can crash or assert if given a value-dependent expression. If we don't defer these checks in dependent contexts, you'll need to handle that possibility somehow.
> > 
> > Example:
> > 
> >   template<int N> void f(const char *p) {
> >     printf("blah blah %s" + N, p);
> >   }
> I think I don't understand what you are trying to tell me. Especially the example you provided does just fine and behaves as I expected. As far as I followed EvaluateAsInt it does not assert but returns false if we don't get a constexpr here. We warn under -Wformat-nonliteral for value-dependent string literals.
> 
> Could you explain this more or provide an example that triggers an assert or explain what behavior is wrong regarding the provided example? Thanks! 
We should not warn for that example, since (for instance) calling `f<0>` is fine (we should warn for `f<11>`, though, since it has no format specifiers).

While `EvaluateAsInt` happens to not assert for that particular value-dependent input, it does assert for some other value-dependent cases. It's not easy for me to find you such a case, because Clang is currently careful to never call this function on a value-dependent expression, but perhaps this will trigger an assert:

  struct S { constexpr S(int n) : n(n) {} int n; };
  template<int N> void f(const char *p) {
    printf("blah blah %s" + S(N).n, p);
  }


https://reviews.llvm.org/D23820