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

Jake Merdich via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jun 26 07:05:11 PDT 2020


JakeMerdichAMD marked 10 inline comments as done.
JakeMerdichAMD added inline comments.


================
Comment at: clang/docs/ClangFormatStyleOptions.rst:2706
+
+  For example: STRINGIZE
+
----------------
curdeius wrote:
> 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.
> ```
> 
Done. I also added PP_STRINGIZE and BOOST_PP_STRINGIZE as defaults; seems reasonable.


================
Comment at: clang/unittests/Format/FormatTest.cpp:13961
+  Style.WhitespaceSensitiveMacros.clear();
+  CHECK_PARSE("WhitespaceSensitiveMacros: [STRINGIZE]",
+              WhitespaceSensitiveMacros, std::vector<std::string>{"STRINGIZE"});
----------------
curdeius wrote:
> Shouldn't that be:
> `CHECK_PARSE("WhitespaceSensitiveMacros: ['STRINGIZE']",`
> as in other options that take vector of strings?
I'll add it since it ought to be tested, but they aren't required to my knowledge due to 'YAML reasons' and most of the other tests omit them.


================
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));
----------------
curdeius wrote:
> How about a test with escaped parentheses `\(` inside the macro argument?
Done. Note that parens always need to be matched anyhow, and the escaping doesn't actually mean anything outside of a string literal (tested on MSVC, gcc, clang), so no functionality change is needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82620





More information about the cfe-commits mailing list