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-format] Don't sort qualifiers across preprocessor directives #81958

Merged
merged 1 commit into from
Feb 17, 2024

Conversation

owenca
Copy link
Contributor

@owenca owenca commented Feb 16, 2024

Fixes #80579.

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-clang-format

Author: Owen Pan (owenca)

Changes

Fixes #80579.


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

2 Files Affected:

  • (modified) clang/lib/Format/QualifierAlignmentFixer.cpp (+2)
  • (modified) clang/unittests/Format/QualifierFixerTest.cpp (+13)
diff --git a/clang/lib/Format/QualifierAlignmentFixer.cpp b/clang/lib/Format/QualifierAlignmentFixer.cpp
index 84941746f0df71..0c63d330b5aed4 100644
--- a/clang/lib/Format/QualifierAlignmentFixer.cpp
+++ b/clang/lib/Format/QualifierAlignmentFixer.cpp
@@ -561,6 +561,8 @@ void LeftRightQualifierAlignmentFixer::fixQualifierAlignment(
 
     for (const auto *Tok = First; Tok && Tok != Last && Tok->Next;
          Tok = Tok->Next) {
+      if (Tok->MustBreakBefore)
+        break;
       if (Tok->is(tok::comment))
         continue;
       if (RightAlign) {
diff --git a/clang/unittests/Format/QualifierFixerTest.cpp b/clang/unittests/Format/QualifierFixerTest.cpp
index 0aa755acfc8213..43476aea66337b 100644
--- a/clang/unittests/Format/QualifierFixerTest.cpp
+++ b/clang/unittests/Format/QualifierFixerTest.cpp
@@ -1177,6 +1177,19 @@ TEST_F(QualifierFixerTest, DontPushQualifierThroughNonSpecifiedTypes) {
   verifyFormat("const inline static Foo;", Style);
 }
 
+TEST_F(QualifierFixerTest, QualifiersBrokenUpByPPDirectives) {
+  auto Style = getLLVMStyle();
+  Style.QualifierAlignment = FormatStyle::QAS_Custom;
+  Style.QualifierOrder = {"constexpr", "inline", "type"};
+
+  verifyFormat("inline\n"
+               "#if FOO\n"
+               "    constexpr\n"
+               "#endif\n"
+               "    int i = 0;",
+               Style);
+}
+
 TEST_F(QualifierFixerTest, UnsignedQualifier) {
 
   FormatStyle Style = getLLVMStyle();

Copy link
Contributor

@mydeveloperday mydeveloperday left a comment

Choose a reason for hiding this comment

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

LGTM, as long as someone doesn't say it should be

#if FOO
   constexpr
#endif
   inline
   int i = 0;

but I think this change is fine..I would rather we didn't try and support that.. thank you.

@owenca
Copy link
Contributor Author

owenca commented Feb 17, 2024

LGTM, as long as someone doesn't say it should be

#if FOO
   constexpr
#endif
   inline
   int i = 0;

but I think this change is fine..I would rather we didn't try and support that.. thank you.

Yep! Otherwise, you would have to handle the following, given that the order is [const, inline, constexpr, type]:

inline
#if FOO
    constexpr /* foo */
#else
    const /* bar */
#endif
    int i = 0;

@owenca owenca merged commit ea16a3b into llvm:main Feb 17, 2024
5 of 6 checks passed
@owenca owenca deleted the qualifiers branch February 17, 2024 07:19
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.

[clang-format] When processing constexpr inline, the middle macro will be deleted
3 participants