[PATCH] D83296: [clang-format] Add a MacroExpander.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added a comment.

In D83296#2299062 <https://reviews.llvm.org/D83296#2299062>, @nridge wrote:

> What does this change mean for users of clang-format -- better formatting of complicated (e.g. multi-line) macro invocations?

In addition to what Sam said, this also attempts to be an improvement in maintainability. Given this is a fairly complex change, you might ask how this helps :)
The idea is that we bundle the complexity of macro handling in a clearly separated part of the code that can be tested and developed ~on its own.
Currently, we have multiple macro regex settings that then lead to random code throughout clang-format that tries to handle those identifiers special.
Once this is done, we can delete all those settings, as the more generalized macro configuration will supersede them.



================
Comment at: clang/lib/Format/MacroExpander.cpp:190
+      return true;
+    for (const auto &Arg : Args[I->getValue()]) {
+      // A token can be part of a macro argument at multiple levels.
----------------
sammccall wrote:
> nit: this is confusingly a const reference to a non-const pointer... `auto *` or `FormatToken *`?
Yikes, thanks for catching!


================
Comment at: clang/unittests/Format/MacroExpanderTest.cpp:183
+  EXPECT_ATTRIBUTES(Result, Attributes);
+}
+
----------------
sammccall wrote:
> may want a test that uses of an arg after the first are not expanded, because that "guards" a bunch of nasty potential bugs
Discussed offline: the above test tests exactly this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83296