[clang] bc840b2 - [Diagnostic] add a warning which warns about misleading indentation
via cfe-commits
cfe-commits at lists.llvm.org
Tue Dec 3 12:22:29 PST 2019
Author: Tyker
Date: 2019-12-03T21:21:27+01:00
New Revision: bc840b21e1612adb603b6c17be0329e3737bb943
URL: https://github.com/llvm/llvm-project/commit/bc840b21e1612adb603b6c17be0329e3737bb943
DIFF: https://github.com/llvm/llvm-project/commit/bc840b21e1612adb603b6c17be0329e3737bb943.diff
LOG: [Diagnostic] add a warning which warns about misleading indentation
Summary: Add a warning for misleading indentation similar to GCC's -Wmisleading-indentation
Reviewers: aaron.ballman, xbolva00
Reviewed By: aaron.ballman, xbolva00
Subscribers: tstellar, cfe-commits, arphaman, Ka-Ka, thakis
Tags: #clang
Differential Revision: https://reviews.llvm.org/D70638
Added:
clang/test/Parser/warn-misleading-indentation.cpp
Modified:
clang/include/clang/Basic/DiagnosticGroups.td
clang/include/clang/Basic/DiagnosticParseKinds.td
clang/include/clang/Lex/Preprocessor.h
clang/include/clang/Parse/Parser.h
clang/lib/Parse/ParseStmt.cpp
clang/test/Index/pragma-diag-reparse.c
clang/test/Misc/warning-wall.c
clang/test/Preprocessor/pragma_diagnostic_sections.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 35e939fda95c..478b217a19f6 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -693,6 +693,7 @@ def ZeroLengthArray : DiagGroup<"zero-length-array">;
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 @@ def Consumed : DiagGroup<"consumed">;
// 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]>;
diff --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index c94d1b99d0e8..8643e1b867f7 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -61,6 +61,13 @@ def warn_null_statement : Warning<
"remove unnecessary ';' to silence this warning">,
InGroup<ExtraSemiStmt>, DefaultIgnore;
+def warn_misleading_indentation : Warning<
+ "misleading indentation; statement is not part of "
+ "the previous '%select{if|else|for|while|else if}0'">,
+ InGroup<MisleadingIndentation>, 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 "
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index e2ddc80d503f..9716196b95c2 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -932,6 +932,12 @@ class Preprocessor {
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;
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index ea1116ff7a23..a1e7bbba9b8e 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -1122,6 +1122,11 @@ class Parser : public CodeCompletionHandler {
/// point for skipping past a simple-declaration.
void SkipMalformedDecl();
+ /// The location of the first statement inside an else that might
+ /// have a missleading indentation. If there is no
+ /// MisleadingIndentationChecker on an else active, this location is invalid.
+ SourceLocation MisleadingIndentationElseLoc;
+
private:
//===--------------------------------------------------------------------===//
// Lexing and parsing of C++ inline methods.
diff --git a/clang/lib/Parse/ParseStmt.cpp b/clang/lib/Parse/ParseStmt.cpp
index 727ab75adae8..dc951dc22f55 100644
--- a/clang/lib/Parse/ParseStmt.cpp
+++ b/clang/lib/Parse/ParseStmt.cpp
@@ -1191,6 +1191,59 @@ bool Parser::ParseParenExprOrCondition(StmtResult *InitStmt,
return false;
}
+namespace {
+
+enum MisleadingStatementKind { MSK_if, MSK_else, MSK_for, MSK_while };
+
+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)) {
+ if (!P.MisleadingIndentationElseLoc.isInvalid()) {
+ StmtLoc = P.MisleadingIndentationElseLoc;
+ P.MisleadingIndentationElseLoc = SourceLocation();
+ }
+ if (Kind == MSK_else && !ShouldSkip)
+ P.MisleadingIndentationElseLoc = SL;
+ }
+ void Check() {
+ NeedsChecking = false;
+ Token Tok = P.getCurToken();
+ if (ShouldSkip || NumDirectives != P.getPreprocessor().getNumDirectives() ||
+ Tok.isOneOf(tok::semi, tok::r_brace) || Tok.isAnnotation() ||
+ Tok.getLocation().isMacroID() || PrevLoc.isMacroID() ||
+ StmtLoc.isMacroID() ||
+ (Kind == MSK_else && P.MisleadingIndentationElseLoc.isInvalid())) {
+ P.MisleadingIndentationElseLoc = SourceLocation();
+ 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()) && SM.getPresumedLineNumber(StmtLoc) !=
+ SM.getPresumedLineNumber(Tok.getLocation())) {
+ P.Diag(Tok.getLocation(), diag::warn_misleading_indentation)
+ << Kind;
+ P.Diag(StmtLoc, diag::note_previous_statement);
+ }
+ }
+};
+
+}
/// ParseIfStatement
/// if-statement: [C99 6.8.4.1]
@@ -1265,6 +1318,8 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
//
ParseScope InnerScope(this, Scope::DeclScope, C99orCXX, Tok.is(tok::l_brace));
+ MisleadingIndentationChecker MIChecker(*this, MSK_if, IfLoc);
+
// Read the 'then' stmt.
SourceLocation ThenStmtLoc = Tok.getLocation();
@@ -1278,6 +1333,9 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
ThenStmt = ParseStatement(&InnerStatementTrailingElseLoc);
}
+ if (Tok.isNot(tok::kw_else))
+ MIChecker.Check();
+
// Pop the 'if' scope if needed.
InnerScope.Exit();
@@ -1305,12 +1363,17 @@ StmtResult Parser::ParseIfStatement(SourceLocation *TrailingElseLoc) {
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 (ElseStmt.isUsable())
+ MIChecker.Check();
+
// Pop the 'else' scope if needed.
InnerScope.Exit();
} else if (Tok.is(tok::code_completion)) {
@@ -1484,9 +1547,13 @@ StmtResult Parser::ParseWhileStatement(SourceLocation *TrailingElseLoc) {
//
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));
+ if (Body.isUsable())
+ MIChecker.Check();
// Pop the body scope if needed.
InnerScope.Exit();
WhileScope.Exit();
@@ -1918,9 +1985,14 @@ StmtResult Parser::ParseForStatement(SourceLocation *TrailingElseLoc) {
if (C99orCXXorObjC)
getCurScope()->decrementMSManglingNumber();
+ MisleadingIndentationChecker MIChecker(*this, MSK_for, ForLoc);
+
// Read the body statement.
StmtResult Body(ParseStatement(TrailingElseLoc));
+ if (Body.isUsable())
+ MIChecker.Check();
+
// Pop the body scope if needed.
InnerScope.Exit();
diff --git a/clang/test/Index/pragma-diag-reparse.c b/clang/test/Index/pragma-diag-reparse.c
index 71d0618d7092..aa1413cda089 100644
--- a/clang/test/Index/pragma-diag-reparse.c
+++ b/clang/test/Index/pragma-diag-reparse.c
@@ -11,6 +11,7 @@ int main (int argc, const char * argv[])
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 \
diff --git a/clang/test/Misc/warning-wall.c b/clang/test/Misc/warning-wall.c
index fadcceefe297..2b27b67eafa1 100644
--- a/clang/test/Misc/warning-wall.c
+++ b/clang/test/Misc/warning-wall.c
@@ -90,6 +90,7 @@ CHECK-NEXT: -Wparentheses-equality
CHECK-NEXT: -Wdangling-else
CHECK-NEXT: -Wswitch
CHECK-NEXT: -Wswitch-bool
+CHECK-NEXT: -Wmisleading-indentation
CHECK-NOT:-W
diff --git a/clang/test/Parser/warn-misleading-indentation.cpp b/clang/test/Parser/warn-misleading-indentation.cpp
new file mode 100644
index 000000000000..e5ed8bba93c1
--- /dev/null
+++ b/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 at -2 {{here}}
+#endif
+ i = i + 1;
+ int x = 0;
+#ifdef WITH_WARN
+// expected-warning at -2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+ return;
+#ifdef CXX17
+ if constexpr (false)
+#ifdef WITH_WARN
+// expected-note at -2 {{here}}
+#endif
+ i = 0;
+ i += 1;
+#ifdef WITH_WARN
+// expected-warning at -2 {{misleading indentation; statement is not part of the previous 'if'}}
+#endif
+#endif
+}
+
+void f1(int i) {
+ for (;i;)
+#ifdef WITH_WARN
+// expected-note at -2 {{here}}
+#endif
+ i = i + 1;
+ i *= 2;
+#ifdef WITH_WARN
+// expected-warning at -2 {{misleading indentation; statement is not part of the previous 'for'}}
+#endif
+ return;
+}
+
+void f2(int i) {
+ while (i)
+#ifdef WITH_WARN
+// expected-note at -2 {{here}}
+#endif
+ i = i + 1; i *= 2;
+#ifdef WITH_WARN
+// expected-warning at -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 at -2 {{here}}
+#endif
+ i *= 2;
+ const int x = 0;
+#ifdef WITH_WARN
+// expected-warning at -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 at -2 {{here}}
+#endif
+ i *= 2;
+ typedef int Int;
+#ifdef WITH_WARN
+// expected-warning at -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 at -2 {{here}}
+#endif
+ i *= 2;
+ using Int2 = int;
+#ifdef WITH_WARN
+// expected-warning at -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 at -3 {{here}}
+#endif
+ i = 4;
+ i = 5;
+#ifdef WITH_WARN
+// expected-warning at -2 {{misleading indentation; statement is not part of the previous '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 at -2 {{here}}
+#endif
+ i = 4;
+ i = 5;
+#ifdef WITH_WARN
+// expected-warning at -2 {{misleading indentation; statement is not part of the previous '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 at -2 {{here}}
+#endif
+ i = 4;
+ i = 5;
+#ifdef WITH_WARN
+// expected-warning at -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;
+}
\ No newline at end of file
diff --git a/clang/test/Preprocessor/pragma_diagnostic_sections.cpp b/clang/test/Preprocessor/pragma_diagnostic_sections.cpp
index b680fae5b993..2bba7595a810 100644
--- a/clang/test/Preprocessor/pragma_diagnostic_sections.cpp
+++ b/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 {
More information about the cfe-commits
mailing list