Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[clang-tidy][misleading-indentation]ignore false-positives for line started with empty macro #75061

Merged
merged 1 commit into from
Dec 27, 2023
Merged

[clang-tidy][misleading-indentation]ignore false-positives for line started with empty macro #75061

merged 1 commit into from
Dec 27, 2023

Conversation

HerrCai0907
Copy link
Contributor

Fixes: #71767

@llvmbot
Copy link
Collaborator

llvmbot commented Dec 11, 2023

@llvm/pr-subscribers-clang-tidy

Author: Congcong Cai (HerrCai0907)

Changes

Fixes: #71767


Full diff: https://github.com/llvm/llvm-project/pull/75061.diff

4 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp (+21-3)
  • (modified) clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.h (+2-1)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+4)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation.cpp (+9)
diff --git a/clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp b/clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
index 2c011f5c0e6904..a096c0e383130a 100644
--- a/clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
+++ b/clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "MisleadingIndentationCheck.h"
+#include "../utils/LexerUtils.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 
@@ -51,8 +52,22 @@ void MisleadingIndentationCheck::danglingElseCheck(const SourceManager &SM,
     diag(ElseLoc, "different indentation for 'if' and corresponding 'else'");
 }
 
-void MisleadingIndentationCheck::missingBracesCheck(const SourceManager &SM,
-                                                    const CompoundStmt *CStmt) {
+static bool isAtStartOfLineIncludingEmptyMacro(SourceLocation NextLoc,
+                                               const SourceManager &SM,
+                                               const LangOptions &LangOpts) {
+  NextLoc.dump(SM);
+  const SourceLocation BeforeLoc =
+      utils::lexer::getPreviousTokenAndStart(NextLoc, SM, LangOpts).second;
+  if (SM.getExpansionLineNumber(BeforeLoc) ==
+      SM.getExpansionLineNumber(NextLoc)) {
+    return false;
+  }
+  return true;
+}
+
+void MisleadingIndentationCheck::missingBracesCheck(
+    const SourceManager &SM, const CompoundStmt *CStmt,
+    const LangOptions &LangOpts) {
   const static StringRef StmtNames[] = {"if", "for", "while"};
   for (unsigned int I = 0; I < CStmt->size() - 1; I++) {
     const Stmt *CurrentStmt = CStmt->body_begin()[I];
@@ -92,6 +107,8 @@ void MisleadingIndentationCheck::missingBracesCheck(const SourceManager &SM,
 
     if (NextLoc.isInvalid() || NextLoc.isMacroID())
       continue;
+    if (!isAtStartOfLineIncludingEmptyMacro(NextLoc, SM, LangOpts))
+      continue;
 
     if (SM.getExpansionColumnNumber(InnerLoc) ==
         SM.getExpansionColumnNumber(NextLoc)) {
@@ -117,7 +134,8 @@ void MisleadingIndentationCheck::check(const MatchFinder::MatchResult &Result) {
     danglingElseCheck(*Result.SourceManager, Result.Context, If);
 
   if (const auto *CStmt = Result.Nodes.getNodeAs<CompoundStmt>("compound"))
-    missingBracesCheck(*Result.SourceManager, CStmt);
+    missingBracesCheck(*Result.SourceManager, CStmt,
+                       Result.Context->getLangOpts());
 }
 
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.h b/clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.h
index c336abbc7c4a91..9c92fc1e18b6f3 100644
--- a/clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.h
+++ b/clang-tools-extra/clang-tidy/readability/MisleadingIndentationCheck.h
@@ -32,7 +32,8 @@ class MisleadingIndentationCheck : public ClangTidyCheck {
 private:
   void danglingElseCheck(const SourceManager &SM, ASTContext *Context,
                          const IfStmt *If);
-  void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt);
+  void missingBracesCheck(const SourceManager &SM, const CompoundStmt *CStmt,
+                          const LangOptions &LangOpts);
 };
 
 } // namespace clang::tidy::readability
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
index 6d91748e4cef18..85438ce795b34c 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -453,6 +453,10 @@ Changes in existing checks
   <clang-tidy/checks/readability/non-const-parameter>` check to ignore
   false-positives in initializer list of record.
 
+- Improved :doc:`readability-misleading-indentation
+  <clang-tidy/checks/readability/misleading-indentation>` check to ignore
+  false-positives for line started with empty macro.
+
 - Improved :doc:`readability-static-accessed-through-instance
   <clang-tidy/checks/readability/static-accessed-through-instance>` check to
   identify calls to static member functions with out-of-class inline definitions.
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation.cpp
index aea0618d120db6..5d4d60f5f1a351 100644
--- a/clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation.cpp
+++ b/clang-tools-extra/test/clang-tidy/checkers/readability/misleading-indentation.cpp
@@ -4,6 +4,8 @@ void foo1();
 void foo2();
 void foo3();
 
+#define E
+
 #define BLOCK \
   if (cond1)  \
     foo1();   \
@@ -109,6 +111,13 @@ void f()
   }
 
   BLOCK
+
+  if (cond1)
+    foo1();
+  else
+    foo2();
+  E foo3();
+  // CHECK-MESSAGES-NOT: :[[@LINE-1]]readability-misleading-indentation
 }
 
 void g(bool x) {

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+- fine, fix pointed out nits and could land

Copy link
Member

@PiotrZSL PiotrZSL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@HerrCai0907 HerrCai0907 merged commit 72390c5 into llvm:main Dec 27, 2023
5 checks passed
@HerrCai0907 HerrCai0907 deleted the fix/misleading-indentation branch December 27, 2023 08:17
@HerrCai0907 HerrCai0907 restored the fix/misleading-indentation branch December 27, 2023 08:17
@HerrCai0907 HerrCai0907 deleted the fix/misleading-indentation branch December 27, 2023 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[readability-misleading-indentation] False report for macro statement
4 participants