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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
