rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.

This basically looks fine to me now. I'm not 100% sold on 
`sumUpStringLiteralOffset` being the best name, but I think we have better 
things to worry about, and it's good enough. Just a couple of minor style 
issues then I think this is good to go.


================
Comment at: lib/Sema/SemaChecking.cpp:3866-3867
@@ +3865,4 @@
+    ResOffset = Offset.ssub_ov(Addend, Ov);
+  else
+    assert(false && "operator must be add or sub with addend on the right");
+
----------------
Rather than `assert(false && XXX);`, use either `llvm_unreachable(XXX)` or 
change the previous case to be:

  else {
    assert(AddendIsRight && BinOpKind == BO_Sub &&
           "operator must be ...");

================
Comment at: lib/Sema/SemaChecking.cpp:4150-4151
@@ +4149,4 @@
+      }
+      FormatStringLiteral FStr =
+          FormatStringLiteral(StrE, Offset.sextOrTrunc(64).getSExtValue());
+      CheckFormatString(S, &FStr, E, Args, HasVAListArg, format_idx,
----------------
You can write this more simply as

  FormatStringLiteral FStr(StrE, Offset.sextOrTrunc(64).getSExtValue());


https://reviews.llvm.org/D23820



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to