Tyker updated this revision to Diff 231534.
Tyker added a comment.
yeah i saw. this version of the patch builds all llvm subproject(not tested on
llgo) without warnings. there is only one case in llvm's code in which this
warning is more agressive thant GCC's.
if (1)
i = 0;
// comment
i = 1; // clang + this patch warns through the comment wereas gcc doesn't.
i don't believe a warning in this case is too bad.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70638/new/
https://reviews.llvm.org/D70638
Files:
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/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/test/Index/pragma-diag-reparse.c
clang/test/Misc/warning-wall.c
clang/test/Parser/warn-misleading-indentation.cpp
clang/test/Preprocessor/pragma_diagnostic_sections.cpp
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 {
Index: clang/test/Parser/warn-misleading-indentation.cpp
===================================================================
--- /dev/null
+++ clang/test/Parser/warn-misleading-indentation.cpp
@@ -0,0 +1,210 @@
+// 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; declaration 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; declaration 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; declaration 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; declaration 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;
+}
+
+// No diagnostics from GCC on this
+void g(int i) {
+ if (1)
+ i = 2;
+ else
+ if (i == 3)
+ i = 4;
+ i = 5;
+}
+
+// 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
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+ if (i == 3)
+ i = 4;
+ i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'else'}}
+#endif
+}
+
+void g2(int i) {
+ if (1)
+ i = 2;
+ else
+#ifdef WITH_WARN
+// expected-note@-2 {{here}}
+#endif
+ if (i == 3)
+ {i = 4;}
+ i = 5;
+#ifdef WITH_WARN
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'else'}}
+#endif
+}
+
+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;
+}
\ No newline at end of file
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/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/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
===================================================================
--- clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
@@ -2525,19 +2525,18 @@
// Find the most recent expression bound to the symbol in the current
// context.
- if (!ReferenceRegion) {
- if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) {
- SVal Val = State->getSVal(MR);
- if (Val.getAsLocSymbol() == Sym) {
- const VarRegion* VR = MR->getBaseRegion()->getAs<VarRegion>();
- // Do not show local variables belonging to a function other than
- // where the error is reported.
- if (!VR ||
- (VR->getStackFrame() == LeakContext->getStackFrame()))
- ReferenceRegion = MR;
- }
+ if (!ReferenceRegion) {
+ if (const MemRegion *MR = C.getLocationRegionIfPostStore(N)) {
+ SVal Val = State->getSVal(MR);
+ if (Val.getAsLocSymbol() == Sym) {
+ const VarRegion *VR = MR->getBaseRegion()->getAs<VarRegion>();
+ // Do not show local variables belonging to a function other than
+ // where the error is reported.
+ if (!VR || (VR->getStackFrame() == LeakContext->getStackFrame()))
+ ReferenceRegion = MR;
}
}
+ }
// Allocation node, is the last node in the current or parent context in
// which the symbol was tracked.
Index: clang/lib/Parse/ParseStmt.cpp
===================================================================
--- clang/lib/Parse/ParseStmt.cpp
+++ clang/lib/Parse/ParseStmt.cpp
@@ -1191,6 +1191,54 @@
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 HasBraces;
+ MisleadingIndentationChecker(Parser &P, MisleadingStatementKind K,
+ SourceLocation SL)
+ : P(P), StmtLoc(SL), PrevLoc(P.getCurToken().getLocation()),
+ NumDirectives(P.getPreprocessor().getNumDirectives()), Kind(K),
+ NeedsChecking(true), HasBraces(P.getCurToken().is(tok::l_brace)) {}
+ void Check(bool ShouldCheck) {
+ NeedsChecking = false;
+ Token Tok = P.getCurToken();
+ if (!ShouldCheck || HasBraces ||
+ 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)
+ << P.isCXXDeclarationStatement() << Kind;
+ P.Diag(StmtLoc, diag::note_previous_statement);
+ }
+ }
+ ~MisleadingIndentationChecker() {
+ assert(!NeedsChecking && "Check Has not been called");
+ }
+};
+
+}
/// ParseIfStatement
/// if-statement: [C99 6.8.4.1]
@@ -1265,6 +1313,8 @@
//
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 +1328,8 @@
ThenStmt = ParseStatement(&InnerStatementTrailingElseLoc);
}
+ MIChecker.Check(Tok.isNot(tok::kw_else));
+
// Pop the 'if' scope if needed.
InnerScope.Exit();
@@ -1305,12 +1357,16 @@
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();
+ MIChecker.Check(ElseStmt.isUsable());
+
// Pop the 'else' scope if needed.
InnerScope.Exit();
} else if (Tok.is(tok::code_completion)) {
@@ -1484,9 +1540,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 +1977,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/include/clang/Parse/Parser.h
===================================================================
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -2266,11 +2266,13 @@
return isTypeSpecifierQualifier();
}
+public:
/// isCXXDeclarationStatement - C++-specialized function that disambiguates
/// between a declaration or an expression statement, when parsing function
/// bodies. Returns true for declaration, false for expression.
bool isCXXDeclarationStatement();
+private:
/// isCXXSimpleDeclaration - C++-specialized function that disambiguates
/// between a simple-declaration or an expression-statement.
/// If during the disambiguation process a parsing error is encountered,
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/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<ExtraSemiStmt>, DefaultIgnore;
+def warn_misleading_indentation : Warning<
+ "misleading indentation; %select{statement|declaration}0 is not part of "
+ "the previous '%select{if|else|for|while}1'">,
+ 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 "
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]>;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
