Index: clang/include/clang/Basic/DiagnosticGroups.td =================================================================== --- clang/include/clang/Basic/DiagnosticGroups.td +++ clang/include/clang/Basic/DiagnosticGroups.td @@ -693,6 +693,7 @@ def GNUZeroLineDirective : DiagGroup<"gnu-zero-line-directive">; def GNUZeroVariadicMacroArguments : DiagGroup<"gnu-zero-variadic-macro-arguments">; def Fallback : DiagGroup<"fallback">; +def MisleadingIndentation : DiagGroup<"misleading-indentation">; // This covers both the deprecated case (in C++98) // and the extension case (in C++11 onwards). @@ -884,7 +885,7 @@ // Note that putting warnings in -Wall will not disable them by default. If a // warning should be active _only_ when -Wall is passed in, mark it as // DefaultIgnore in addition to putting it here. -def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool]>; +def All : DiagGroup<"all", [Most, Parentheses, Switch, SwitchBool, MisleadingIndentation]>; // Warnings that should be in clang-cl /w4. def : DiagGroup<"CL4", [All, Extra]>; Index: clang/include/clang/Basic/DiagnosticParseKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticParseKinds.td +++ clang/include/clang/Basic/DiagnosticParseKinds.td @@ -61,6 +61,13 @@ "remove unnecessary ';' to silence this warning">, InGroup, DefaultIgnore; +def warn_misleading_indentation : Warning< + "misleading indentation; statement is not part of " + "the previous '%select{if|else|for|while|else if}0'">, + InGroup, DefaultIgnore; +def note_previous_statement : Note< + "previous statement is here">; + def ext_thread_before : Extension<"'__thread' before '%0'">; def ext_keyword_as_ident : ExtWarn< "keyword '%0' will be made available as an identifier " Index: clang/include/clang/Lex/Preprocessor.h =================================================================== --- clang/include/clang/Lex/Preprocessor.h +++ clang/include/clang/Lex/Preprocessor.h @@ -932,6 +932,12 @@ return TheModuleLoader.HadFatalFailure; } + /// Retrieve the number of Directives that have been processed by the + /// Preprocessor. + unsigned getNumDirectives() const { + return NumDirectives; + } + /// True if we are currently preprocessing a #if or #elif directive bool isParsingIfOrElifDirective() const { return ParsingIfOrElifDirective; Index: clang/include/clang/Parse/Parser.h =================================================================== --- clang/include/clang/Parse/Parser.h +++ clang/include/clang/Parse/Parser.h @@ -1961,7 +1961,9 @@ Sema::ConditionResult &CondResult, SourceLocation Loc, Sema::ConditionKind CK); - StmtResult ParseIfStatement(SourceLocation *TrailingElseLoc); + StmtResult + ParseIfStatement(SourceLocation *TrailingElseLoc, + SourceLocation ElseLocOnElseIf = SourceLocation()); StmtResult ParseSwitchStatement(SourceLocation *TrailingElseLoc); StmtResult ParseWhileStatement(SourceLocation *TrailingElseLoc); StmtResult ParseDoStatement(); Index: clang/lib/Parse/ParseStmt.cpp =================================================================== --- clang/lib/Parse/ParseStmt.cpp +++ clang/lib/Parse/ParseStmt.cpp @@ -1191,6 +1191,56 @@ return false; } +namespace { + +enum MisleadingStatementKind { MSK_if, MSK_else, MSK_for, MSK_while, MSK_else_if }; + +struct MisleadingIndentationChecker { + Parser &P; + SourceLocation StmtLoc; + SourceLocation PrevLoc; + unsigned NumDirectives; + MisleadingStatementKind Kind; + bool NeedsChecking; + bool ShouldSkip; + MisleadingIndentationChecker(Parser &P, MisleadingStatementKind K, + SourceLocation SL) + : P(P), StmtLoc(SL), PrevLoc(P.getCurToken().getLocation()), + NumDirectives(P.getPreprocessor().getNumDirectives()), Kind(K), + NeedsChecking(true), + ShouldSkip(P.getCurToken().is(tok::l_brace) || + (K == MSK_else && P.getCurToken().is(tok::kw_if))) {} + void Check(bool ShouldCheck) { + NeedsChecking = false; + Token Tok = P.getCurToken(); + if (!ShouldCheck || ShouldSkip || + NumDirectives != P.getPreprocessor().getNumDirectives() || + Tok.isOneOf(tok::semi, tok::r_brace) || Tok.isAnnotation() || + Tok.getLocation().isMacroID() || PrevLoc.isMacroID() || + StmtLoc.isMacroID()) + return; + SourceManager &SM = P.getPreprocessor().getSourceManager(); + unsigned PrevColNum = SM.getSpellingColumnNumber(PrevLoc); + unsigned CurColNum = SM.getSpellingColumnNumber(Tok.getLocation()); + unsigned StmtColNum = SM.getSpellingColumnNumber(StmtLoc); + + if (PrevColNum != 0 && CurColNum != 0 && StmtColNum != 0 && + ((PrevColNum > StmtColNum && PrevColNum == CurColNum) || + !Tok.isAtStartOfLine())) { + if (SM.getPresumedLineNumber(StmtLoc) == + SM.getPresumedLineNumber(Tok.getLocation())) + return; + P.Diag(Tok.getLocation(), diag::warn_misleading_indentation) + << Kind; + P.Diag(StmtLoc, diag::note_previous_statement); + } + } + ~MisleadingIndentationChecker() { + assert(!NeedsChecking && "Check Has not been called"); + } +}; + +} /// ParseIfStatement /// if-statement: [C99 6.8.4.1] @@ -1199,7 +1249,8 @@ /// [C++] 'if' '(' condition ')' statement /// [C++] 'if' '(' condition ')' statement 'else' statement /// -StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) { +StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc, + SourceLocation ElseLocOnElseIf) { assert(Tok.is(tok::kw_if) && "Not an if stmt!"); SourceLocation IfLoc = ConsumeToken(); // eat the 'if'. @@ -1265,6 +1316,10 @@ // ParseScope InnerScope(this, Scope::DeclScope, C99orCXX, Tok.is(tok::l_brace)); + MisleadingIndentationChecker MIChecker( + *this, ElseLocOnElseIf.isInvalid() ? MSK_if : MSK_else_if, + ElseLocOnElseIf.isInvalid() ? IfLoc : ElseLocOnElseIf); + // Read the 'then' stmt. SourceLocation ThenStmtLoc = Tok.getLocation(); @@ -1278,6 +1333,8 @@ ThenStmt = ParseStatement(&InnerStatementTrailingElseLoc); } + MIChecker.Check(Tok.isNot(tok::kw_else)); + // Pop the 'if' scope if needed. InnerScope.Exit(); @@ -1305,11 +1362,18 @@ ParseScope InnerScope(this, Scope::DeclScope, C99orCXX, Tok.is(tok::l_brace)); + MisleadingIndentationChecker MIChecker(*this, MSK_else, ElseLoc); + EnterExpressionEvaluationContext PotentiallyDiscarded( Actions, Sema::ExpressionEvaluationContext::DiscardedStatement, nullptr, Sema::ExpressionEvaluationContextRecord::EK_Other, /*ShouldEnter=*/ConstexprCondition && *ConstexprCondition); - ElseStmt = ParseStatement(); + if (Tok.is(tok::kw_if)) + ElseStmt = ParseIfStatement(nullptr, ElseLoc); + else + ElseStmt = ParseStatement(); + + MIChecker.Check(ElseStmt.isUsable()); // Pop the 'else' scope if needed. InnerScope.Exit(); @@ -1484,9 +1548,12 @@ // ParseScope InnerScope(this, Scope::DeclScope, C99orCXX, Tok.is(tok::l_brace)); + MisleadingIndentationChecker MIChecker(*this, MSK_while, WhileLoc); + // Read the body statement. StmtResult Body(ParseStatement(TrailingElseLoc)); + MIChecker.Check(Body.isUsable()); // Pop the body scope if needed. InnerScope.Exit(); WhileScope.Exit(); @@ -1918,9 +1985,13 @@ if (C99orCXXorObjC) getCurScope()->decrementMSManglingNumber(); + MisleadingIndentationChecker MIChecker(*this, MSK_for, ForLoc); + // Read the body statement. StmtResult Body(ParseStatement(TrailingElseLoc)); + MIChecker.Check(Body.isUsable()); + // Pop the body scope if needed. InnerScope.Exit(); Index: clang/test/Index/pragma-diag-reparse.c =================================================================== --- clang/test/Index/pragma-diag-reparse.c +++ clang/test/Index/pragma-diag-reparse.c @@ -11,6 +11,7 @@ return x; } +#pragma clang diagnostic ignored "-Wmisleading-indentation" void foo() { int b=0; while (b==b); } // RUN: env CINDEXTEST_EDITING=1 CINDEXTEST_FAILONERROR=1 c-index-test -test-load-source-reparse 5 local \ Index: clang/test/Misc/warning-wall.c =================================================================== --- clang/test/Misc/warning-wall.c +++ clang/test/Misc/warning-wall.c @@ -90,6 +90,7 @@ CHECK-NEXT: -Wdangling-else CHECK-NEXT: -Wswitch CHECK-NEXT: -Wswitch-bool +CHECK-NEXT: -Wmisleading-indentation CHECK-NOT:-W Index: clang/test/Parser/warn-misleading-indentation.cpp =================================================================== --- /dev/null +++ clang/test/Parser/warn-misleading-indentation.cpp @@ -0,0 +1,208 @@ +// RUN: %clang_cc1 -x c -fsyntax-only -verify %s +// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wmisleading-indentation -DWITH_WARN %s +// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -DWITH_WARN -DCXX17 %s +// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify -Wall -Wno-unused -Wno-misleading-indentation -DCXX17 %s + +#ifndef WITH_WARN +// expected-no-diagnostics +#endif + +void f0(int i) { + if (i) +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i = i + 1; + int x = 0; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}} +#endif + return; +#ifdef CXX17 + if constexpr (false) +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i = 0; + i += 1; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}} +#endif +#endif +} + +void f1(int i) { + for (;i;) +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i = i + 1; + i *= 2; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'for'}} +#endif + return; +} + +void f2(int i) { + while (i) +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i = i + 1; i *= 2; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'while'}} +#endif + return; +} + +void f3(int i) { + if (i) + i = i + 1; + else +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i *= 2; + const int x = 0; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'else'}} +#endif +} + +#ifdef CXX17 +struct Range { + int *begin() {return nullptr;} + int *end() {return nullptr;} +}; +#endif + +void f4(int i) { + if (i) + i *= 2; + return; + if (i) + i *= 2; + ; + if (i) +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i *= 2; + typedef int Int; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}} +#endif +#ifdef CXX17 + Range R; + for (auto e : R) +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i *= 2; + using Int2 = int; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'for'}} +#endif +#endif +} + +int bar(void); + +int foo(int* dst) +{ + if (dst) + return + bar(); + if (dst) + dst = dst + \ + bar(); + return 0; +} + +void g(int i) { + if (1) + i = 2; + else + if (i == 3) +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i = 4; + i = 5; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'else if'}} +#endif +} + +// Or this +#define TEST i = 5 +void g0(int i) { + if (1) + i = 2; + else + i = 5; + TEST; +} + +void g1(int i) { + if (1) + i = 2; + else if (i == 3) +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i = 4; + i = 5; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'else if'}} +#endif +} + +void g2(int i) { + if (1) + i = 2; + else + if (i == 3) + {i = 4;} + i = 5; +} + +void g6(int i) { + if (1) + if (i == 3) +#ifdef WITH_WARN +// expected-note@-2 {{here}} +#endif + i = 4; + i = 5; +#ifdef WITH_WARN +// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}} +#endif +} + +void g7(int i) { + if (1) + i = 4; +#ifdef TEST1 +#endif + i = 5; +} + +void a1(int i) { if (1) i = 4; return; } + +void a2(int i) { + { + if (1) + i = 4; + } + return; +} + +void a3(int i) { + if (1) + { + i = 4; + } + return; +} Index: clang/test/Preprocessor/pragma_diagnostic_sections.cpp =================================================================== --- clang/test/Preprocessor/pragma_diagnostic_sections.cpp +++ clang/test/Preprocessor/pragma_diagnostic_sections.cpp @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -fsyntax-only -Wall -Wunused-macros -Wunused-parameter -Wno-uninitialized -verify %s +// RUN: %clang_cc1 -fsyntax-only -Wall -Wunused-macros -Wunused-parameter -Wno-uninitialized -Wno-misleading-indentation -verify %s // rdar://8365684 struct S {