[PATCH] D82620: [clang-format] Preserve whitespace in selected macros

[Marek Kurdej via Phabricator via cfe-commits]

curdeius added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:2706
+
+  For example: STRINGIZE
+
----------------
Shouldn't there be a configuration example like what's in `ForEachMacros` doc?
```
  In the .clang-format configuration file, this can be configured like:

  .. code-block:: yaml

    WhitespaceSensitiveMacros: ['STRINGIZE', 'PP_STRINGIZE']

  For example: BOOST_PP_STRINGIZE.
```



================
Comment at: clang/lib/Format/FormatTokenLexer.cpp:46-49
+  for (const std::string &WhitespaceSensitiveMacro :
+       Style.WhitespaceSensitiveMacros)
+    Macros.insert(
+        {&IdentTable.get(WhitespaceSensitiveMacro), TT_UntouchableMacroFunc});
----------------
Personally I would add braces around the loop body.


================
Comment at: clang/unittests/Format/FormatTest.cpp:13961
+  Style.WhitespaceSensitiveMacros.clear();
+  CHECK_PARSE("WhitespaceSensitiveMacros: [STRINGIZE]",
+              WhitespaceSensitiveMacros, std::vector<std::string>{"STRINGIZE"});
----------------
Shouldn't that be:
`CHECK_PARSE("WhitespaceSensitiveMacros: ['STRINGIZE']",`
as in other options that take vector of strings?


================
Comment at: clang/unittests/Format/FormatTest.cpp:13963
+              WhitespaceSensitiveMacros, std::vector<std::string>{"STRINGIZE"});
+  CHECK_PARSE("WhitespaceSensitiveMacros: [STRINGIZE, ASSERT]",
+              WhitespaceSensitiveMacros,
----------------
Ditto: apostrophes around strings.


================
Comment at: clang/unittests/Format/FormatTest.cpp:16482
+  // and these are all whitespace sensitive by definition
+  EXPECT_EQ("FOO(String-ized&Messy+But(: :Still)=Intentional);",
+            format("FOO(String-ized&Messy+But(: :Still)=Intentional);", Style));
----------------
How about a test with escaped parentheses `\(` inside the macro argument?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82620/new/

https://reviews.llvm.org/D82620