[PATCH] D60362: [clang-format] [PR39719] clang-format converting object-like macro to function-like macro

[MyDeveloperDay via Phabricator via llvm-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: klimek, djasper, reuk, russellmcc.
MyDeveloperDay added a project: clang-tools-extra.

Clang format is incorrectly formatting macros where keywords are redefined

See https://bugs.llvm.org/show_bug.cgi?id=39719

The following code

  - #define true ((foo)1)
  - #define false ((foo)0)

becomes

  + #define true((foo)1)
  + #define false((foo)0)


https://reviews.llvm.org/D60362

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===================================================================
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -13423,6 +13423,25 @@
       guessLanguage("foo.h", "#define FOO ({ foo(); ({ NSString *s; }) })"));
 }
 
+TEST_F(FormatTest, MacroKeyWordsAndParents) {
+
+  format::FormatStyle Style = format::getLLVMStyle();
+  verifyFormat("#define TRUE ((foo)1)", Style);
+  verifyFormat("#define throw ((foo)1)", Style);
+  verifyFormat("#define true ((foo)1)", Style);
+  verifyFormat("#define false ((foo)1)", Style);
+  verifyFormat("#define sizeof ((foo)1)", Style);
+  verifyFormat("#define new ((foo)1)", Style);
+  verifyFormat("#define delete ((foo)1)", Style);
+  verifyFormat("#define for ((foo)1)", Style);
+  verifyFormat("#define override ((foo)1)", Style);
+  verifyFormat("#define else ((foo)1)", Style);
+  verifyFormat("#define true 1", Style);
+  verifyFormat("#define true foo", Style);
+  verifyFormat("#define true foo()", Style);
+  verifyFormat("#define NO_LENGTH (~(uint64_t)0)", Style);
+}
+
 } // end namespace
 } // end namespace format
 } // end namespace clang
Index: clang/lib/Format/TokenAnnotator.cpp
===================================================================
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -2462,6 +2462,13 @@
 bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line,
                                           const FormatToken &Left,
                                           const FormatToken &Right) {
+  // if in a #define and a keyword is being defined e.g. #define true (1)
+  // ensure the space between the keyword and '(' is preserved
+  if (Line.InPPDirective && Right.is(tok::l_paren) &&
+      !Left.is(tok::identifier) && Left.Previous &&
+      Left.Previous->is(tok::identifier) && Left.Previous->Previous &&
+      Left.Previous->Previous->is(tok::hash))
+    return true;
   if (Left.is(tok::kw_return) && Right.isNot(tok::semi))
     return true;
   if (Left.is(Keywords.kw_assert) && Style.Language == FormatStyle::LK_Java)


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D60362.194016.patch
Type: text/x-patch
Size: 2139 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190406/c8cc897a/attachment.bin>