Tyker updated this revision to Diff 231545.
Tyker added a comment.
I just found out that `Parser::isCXXDeclarationStatement` is does more then
just disambiguation it can emit diagnostics. which can cause error on correct
code. so we can't use it in this context to disambiguate.
so it is not possible to easily disambiguate between statements and
declaration. i changed the warning message to reflect that but the message
isn't so good now.
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/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/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,55 @@
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)
+ << 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 +1314,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 +1329,8 @@
ThenStmt = ParseStatement(&InnerStatementTrailingElseLoc);
}
+ MIChecker.Check(Tok.isNot(tok::kw_else));
+
// Pop the 'if' scope if needed.
InnerScope.Exit();
@@ -1305,12 +1358,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 +1541,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 +1978,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; this is not part of "
+ "the previous '%select{if|else|for|while}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 "
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
