rsmith added a comment.
Please also handle expressions of the form `&"str"[i]`. We warn (under
`-Wstring-plus-int`) for `"str" + i` and suggest rewriting into that form, so
we should also handle that form here.
================
Comment at: lib/Sema/SemaChecking.cpp:3864
@@ -3864,1 +3863,3 @@
+ UncoveredArgHandler &UncoveredArg,
+ int64_t &Offset) {
tryAgain:
----------------
Why is this passed by reference? It's just an input, not an output, right?
================
Comment at: lib/Sema/SemaChecking.cpp:4062-4067
@@ +4061,8 @@
+ // into the original string literal.
+ StrE = StringLiteral::Create(S.Context,
+ StrE->getString().drop_front(Offset),
+ StrE->getKind(),
+ StrE->isPascal(),
+ StrE->getType(),
+ StrE->getLocStart());
+ } else if (Offset < 0) {
----------------
This doesn't seem like it preserves enough information for the downstream code
to give correct caret diagnostics pointing at locations within the string. It
seems like it would be extremely complicated to maintain the necessary
invariants to make that work (you'd need to create a fake string literal source
buffer so that the `StringLiteralParser` can reparse it, for whichever of the
string literal tokens the offset ends up within). Have you looked at how much
work it'd be to feed a starting offset into `CheckFormatString` instead?
================
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);
+
----------------
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);
}
================
Comment at: lib/Sema/SemaChecking.cpp:4097-4100
@@ +4096,6 @@
+ if (LIsInt) {
+ Offset += LResult.getExtValue();
+ E = BinOp->getRHS();
+ } else {
+ Offset += RResult.getExtValue();
+ E = BinOp->getLHS();
----------------
This will assert if the result doesn't fit into 64 bits, and it's not
guaranteed to (if one of the operands was cast to `__int128`, for instance).
You could use `getLimitedValue` instead, with some suitable limit.
https://reviews.llvm.org/D23820
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
