Commit bc840b21 authored by Tyker's avatar Tyker
Browse files

[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
parent 2f960472

clang/include/clang/Basic/DiagnosticGroups.td

+2 −1
Original line number Diff line number Diff line
@@ -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]>;

clang/include/clang/Basic/DiagnosticParseKinds.td

+7 −0
Original line number Diff line number Diff line
@@ -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 "

clang/include/clang/Lex/Preprocessor.h

+6 −0
Original line number Diff line number Diff line
@@ -932,6 +932,12 @@ public: 
    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;

clang/include/clang/Parse/Parser.h

+5 −0
Original line number Diff line number Diff line
@@ -1122,6 +1122,11 @@ public: 
  /// 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.

clang/lib/Parse/ParseStmt.cpp

+72 −0
Original line number Diff line number Diff line
@@ -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();