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