Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -28,6 +28,16 @@ InGroup, DefaultIgnore; def note_loop_iteration_here : Note<"%select{decremented|incremented}0 here">; +def warn_for_range_string_literal : Warning< + "range-based for-loop working over a string literal will process the " + "trailing null character">, + InGroup, DefaultIgnore; +def note_for_range_string_literal_fix : Note< + "fix by explicitly handling the null character at the beginning of the loop " + "body with an if statement">; +def note_for_range_string_literal_silence : Note< + "place parenthesis around the string literal to silence this warning">; + def warn_duplicate_enum_values : Warning< "element %0 has been implicitly assigned %1 which another element has " "been assigned">, InGroup>, DefaultIgnore; Index: clang/lib/Sema/SemaStmt.cpp =================================================================== --- clang/lib/Sema/SemaStmt.cpp +++ clang/lib/Sema/SemaStmt.cpp @@ -2851,6 +2851,102 @@ } } +// Diagnoses when a string literal is used as the range for a range-based +// for-loop, because the trailing null will unexpected be handled with the +// rest of the characters in the string. The warning can be silenced either +// by add parentheses around the string literal or by explicitly handling +// the null character with an if statement at the beginning of the loop body. +static void DiagnoseForRangeStringLiteral(Sema &SemaRef, + const CXXForRangeStmt *ForStmt) { + if (SemaRef.Diags.isIgnored(diag::warn_for_range_string_literal, + ForStmt->getBeginLoc())) { + return; + } + + const VarDecl *VD = ForStmt->getLoopVariable(); + if (!VD || !VD->getType()->isIntegerType()) + return; + + const Stmt *Init = ForStmt->getRangeInit(); + if (!Init) + return; + + const StringLiteral* Literal = dyn_cast(Init); + if (!Literal) + return; + + const Stmt *FirstStmt = ForStmt->getBody(); + if (!FirstStmt) + return; + + if (const CompoundStmt *CS = dyn_cast(FirstStmt)) { + if (!CS->body_empty()) { + FirstStmt = CS->body_front(); + } + } + + // Returns true if Statement checks that variable Var is a null character. + auto IsNullCharacterCheck = [](const Stmt *Statement, const VarDecl *Var) { + const IfStmt *IS = dyn_cast(Statement); + if (!IS) + return false; + const Expr* Condition = IS->getCond()->IgnoreParenImpCasts(); + + // if (c), direct conversion to bool + if (const DeclRefExpr *DRE = dyn_cast(Condition)) { + return DRE->getDecl() == Var; + } + + // if (c > 0), comparison against 0 or '\0' + const BinaryOperator *BO = dyn_cast(Condition); + if (!BO || !BO->isComparisonOp()) + return false; + + const Expr *LHS = BO->getLHS()->IgnoreParenImpCasts(); + const Expr *RHS = BO->getRHS()->IgnoreParenImpCasts(); + + auto IsNullCharOrZero = [](const Expr *E) { + if (const CharacterLiteral *Character = dyn_cast(E)) { + return Character->getValue() == 0; + } + if (const IntegerLiteral *Integer = dyn_cast(E)) { + return Integer->getValue() == 0; + } + return false; + }; + + auto IsVariable = [](const Expr *E, const VarDecl *Var) { + if (const DeclRefExpr *DRE = dyn_cast(E)) { + return DRE->getDecl() == Var; + } + return false; + }; + + const bool NullCharOrZero = IsNullCharOrZero(LHS) || IsNullCharOrZero(RHS); + const bool Variable = IsVariable(LHS, Var) || IsVariable(RHS, Var); + + if (NullCharOrZero && Variable) return true; + + return false; + }; + + if (IsNullCharacterCheck(FirstStmt, VD)) return; + + SourceRange Range = Literal->getSourceRange(); + SemaRef.Diag(Literal->getExprLoc(), diag::warn_for_range_string_literal) + << Range; + + SemaRef.Diag(FirstStmt->getBeginLoc(), + diag::note_for_range_string_literal_fix); + + SourceLocation Open = Range.getBegin(); + SourceLocation Close = SemaRef.getLocForEndOfToken(Range.getEnd()); + + SemaRef.Diag(Range.getBegin(), + diag::note_for_range_string_literal_silence) + << Range << FixItHint::CreateInsertion(Open, "(") + << FixItHint::CreateInsertion(Close, ")"); +} /// FinishCXXForRangeStmt - Attach the body to a C++0x for-range statement. /// This is a separate step from ActOnCXXForRangeStmt because analysis of the /// body cannot be performed until after the type of the range variable is @@ -2869,6 +2965,7 @@ diag::warn_empty_range_based_for_body); DiagnoseForRangeVariableCopies(*this, ForStmt); + DiagnoseForRangeStringLiteral(*this, ForStmt); return S; } Index: clang/test/SemaCXX/warn-range-loop-analysis.cpp =================================================================== --- clang/test/SemaCXX/warn-range-loop-analysis.cpp +++ clang/test/SemaCXX/warn-range-loop-analysis.cpp @@ -297,3 +297,66 @@ for (int x : I) {} // No warning } + +namespace StringLiteral { +void test1(int x) { + for (int c : "hello world") {} + // expected-warning@-1 {{range-based for-loop working over a string literal will process the trailing null character}} + // expected-note@-2 {{fix by explicitly handling the null character at the beginning of the loop body with an if statement}} + // expected-note@-3 {{place parenthesis around the string literal to silence this warning}} + for (char c : "hello world") {} + // expected-warning@-1 {{range-based for-loop working over a string literal will process the trailing null character}} + // expected-note@-2 {{fix by explicitly handling the null character at the beginning of the loop body with an if statement}} + // expected-note@-3 {{place parenthesis around the string literal to silence this warning}} + + // Various methods of silencing the warning. + for (char c : ("hello world")) {} + for (char c : "hello world") { if (c > 0) {} } + for (char c : "hello world") { if (c == '\0') {} } + for (char c : "hello world") { if (c != 0) {} } + for (char c : "hello world") { if (c) {} } + for (char c : "hello world") if (c > 0) {} + for (char c : "hello world") if (c == '\0') {} + for (char c : "hello world") if (c != 0) {} + for (char c : "hello world") if (c) {} + + // Almost silencing the warning. + for (char c : "hello world") { if (c >> 0) {} } + // expected-warning@-1 {{null character}} + // expected-note@-2 {{if statement}} + // expected-note@-3 {{silence this warning}} + for (char c : "hello world") { if (x > 0) {} } + // expected-warning@-1 {{null character}} + // expected-note@-2 {{if statement}} + // expected-note@-3 {{silence this warning}} + for (char c : "hello world") { if (x == '\0') {} } + // expected-warning@-1 {{null character}} + // expected-note@-2 {{if statement}} + // expected-note@-3 {{silence this warning}} + for (char c : "hello world") { if (x != 0) {} } + // expected-warning@-1 {{null character}} + // expected-note@-2 {{if statement}} + // expected-note@-3 {{silence this warning}} + for (char c : "hello world") { if (x) {} } + // expected-warning@-1 {{null character}} + // expected-note@-2 {{if statement}} + // expected-note@-3 {{silence this warning}} + for (char c : "hello world") if (x > 0) {} + // expected-warning@-1 {{null character}} + // expected-note@-2 {{if statement}} + // expected-note@-3 {{silence this warning}} + for (char c : "hello world") if (x == '\0') {} + // expected-warning@-1 {{null character}} + // expected-note@-2 {{if statement}} + // expected-note@-3 {{silence this warning}} + for (char c : "hello world") if (x != 0) {} + // expected-warning@-1 {{null character}} + // expected-note@-2 {{if statement}} + // expected-note@-3 {{silence this warning}} + for (char c : "hello world") if (x) {} + // expected-warning@-1 {{null character}} + // expected-note@-2 {{if statement}} + // expected-note@-3 {{silence this warning}} +} + +} // namespace StringLiteral