[Diagnostic] Improve discoverability of ftabstop for misleading indentation

[Tyker]

Summary

This adds an additional note after the misleading warning when the code has a mix of tab and space informing about -ftabstop when -ftabstop is used in the generation of the warning.

[Tyker]

i tried to find a good message for the note but i don't think i succeeded. any suggestions.

[jyknight]

clang/include/clang/Basic/DiagnosticParseKinds.td

67

Maybe "assuming tabstops every %0 spaces (override with -ftabstop=)"

clang/include/clang/Basic/DiagnosticParseKinds.td

def warn_null_statement : Warning<
   InGroup<ExtraSemiStmt>, DefaultIgnore;
 
 def warn_misleading_indentation : Warning<
   "misleading indentation; statement 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 note_misleading_indentation_tab_space_mix : Note<
+  "there is a mix of tabs spaces; the tab size (-ftabstop=X) is set to %0">;
 
 def ext_thread_before : Extension<"'__thread' before '%0'">;
 def ext_keyword_as_ident : ExtWarn<
   "keyword '%0' will be made available as an identifier "
   "%select{here|for the remainder of the translation unit}1">,
   InGroup<KeywordCompat>;
 
 def ext_nullability : Extension<

clang/lib/Parse/ParseStmt.cpp

if (!P.MisleadingIndentationElseLoc.isInvalid()) {
       P.MisleadingIndentationElseLoc = SourceLocation();
     }
     if (Kind == MSK_else && !ShouldSkip)
       P.MisleadingIndentationElseLoc = SL;
   }
 
   /// Compute the column number will aligning tabs on TabStop (-ftabstop), this
   /// gives the visual indentation of the SourceLocation.
-  static unsigned getVisualIndentation(SourceManager &SM, SourceLocation Loc) {
+  static unsigned getVisualIndentation(SourceManager &SM, SourceLocation Loc,
+                                       bool& HasTab, bool& HasSpace) {
     unsigned TabStop = SM.getDiagnostics().getDiagnosticOptions().TabStop;
 
     unsigned ColNo = SM.getSpellingColumnNumber(Loc);
-    if (ColNo == 0 || TabStop == 1)
+    if (ColNo == 0)
       return ColNo;
 
     std::pair<FileID, unsigned> FIDAndOffset = SM.getDecomposedLoc(Loc);
 
     bool Invalid;
     StringRef BufData = SM.getBufferData(FIDAndOffset.first, &Invalid);
     if (Invalid)
       return 0;
 
     const char *EndPos = BufData.data() + FIDAndOffset.second;
     // FileOffset are 0-based and Column numbers are 1-based
     assert(FIDAndOffset.second + 1 >= ColNo &&
            "Column number smaller than file offset?");
 
     unsigned VisualColumn = 0; // Stored as 0-based column, here.
     // Loop from beginning of line up to Loc's file position, counting columns,
     // expanding tabs.
     for (const char *CurPos = EndPos - (ColNo - 1); CurPos != EndPos;
          ++CurPos) {
+      HasTab |= *CurPos == '\t';
+      HasSpace |= *CurPos == ' ';
       if (*CurPos == '\t')
         // Advance visual column to next tabstop.
         VisualColumn += (TabStop - VisualColumn % TabStop);
       else
         VisualColumn++;
     }
     return VisualColumn + 1;
   }
if (P.getActions().getDiagnostics().isIgnored(
         (Kind == MSK_else && P.MisleadingIndentationElseLoc.isInvalid())) {
       P.MisleadingIndentationElseLoc = SourceLocation();
       return;
     }
     if (Kind == MSK_else)
       P.MisleadingIndentationElseLoc = SourceLocation();
 
     SourceManager &SM = P.getPreprocessor().getSourceManager();
-    unsigned PrevColNum = getVisualIndentation(SM, PrevLoc);
-    unsigned CurColNum = getVisualIndentation(SM, Tok.getLocation());
-    unsigned StmtColNum = getVisualIndentation(SM, StmtLoc);
+    bool HasTab = false;
+    bool HasSpace = false;
+    unsigned PrevColNum = getVisualIndentation(SM, PrevLoc, HasTab, HasSpace);
+    unsigned CurColNum =
+        getVisualIndentation(SM, Tok.getLocation(), HasTab, HasSpace);
+    unsigned StmtColNum = getVisualIndentation(SM, StmtLoc, HasTab, HasSpace);
 
     if (PrevColNum != 0 && CurColNum != 0 && StmtColNum != 0 &&
         ((PrevColNum > StmtColNum && PrevColNum == CurColNum) ||
          !Tok.isAtStartOfLine()) &&
         SM.getPresumedLineNumber(StmtLoc) !=
             SM.getPresumedLineNumber(Tok.getLocation()) &&
         (Tok.isNot(tok::identifier) ||
          P.getPreprocessor().LookAhead(0).isNot(tok::colon))) {
       P.Diag(Tok.getLocation(), diag::warn_misleading_indentation) << Kind;
+      if (HasTab && HasSpace)
+        P.Diag(Tok.getLocation(),
+               diag::note_misleading_indentation_tab_space_mix)
+            << SM.getDiagnostics().getDiagnosticOptions().TabStop;
       P.Diag(StmtLoc, diag::note_previous_statement);
     }
   }
 };
 
 }
 
 /// ParseIfStatement

clang/test/Parser/warn-misleading-indentation.cpp

 }
 int a4()
 {
   if (0)
     return 1;
   return 0;
 #if (TAB_SIZE == 1)
 // expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
-// expected-note@-5 {{here}}
+// expected-note@-3 {{there is a mix of tabs spaces; the tab size (-ftabstop=X) is set to 1}}
+// expected-note@-6 {{here}}
 #endif 
 }
 
 int a5()
 {
   if (0)
     return 1;
     return 0;
 #if WITH_WARN
 // expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
 // expected-note@-5 {{here}}
 #endif
 }
 
 int a6()
 {
   if (0)
     return 1;
           return 0;
 #if (TAB_SIZE == 8)
 // expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
-// expected-note@-5 {{here}}
+// expected-note@-3 {{there is a mix of tabs spaces; the tab size (-ftabstop=X) is set to 8}}
+// expected-note@-6 {{here}}
+#endif
+}
+
+int a7()
+{
+  if (0)
+    return 1;
+        return 0;
+#if (TAB_SIZE == 4)
+// expected-warning@-2 {{misleading indentation; statement is not part of the previous 'if'}}
+// expected-note@-3 {{there is a mix of tabs spaces; the tab size (-ftabstop=X) is set to 4}}
+// expected-note@-6 {{here}}
 #endif
 }
 
 #define FOO \
  goto fail
 
 int main(int argc, char* argv[]) {
   if (5 != 0)