[PATCH] D144170: [clang-format] Add simple macro replacements in formatting.

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: clang/lib/Format/MacroExpander.cpp:172
   assert(defined(ID->TokenText));
+  assert(
+      (!OptionalArgs && ObjectLike.find(ID->TokenText) != ObjectLike.end()) ||
----------------
sammccall wrote:
> this assertion failure isn't going to be as useful as it could be!
> 
> maybe rather
> ```
> if (OptionalArgs)
>   assert(...);
> else
>   assert(...);
> ```
> 
> Also I think we're now ensuring to only call this if arity matches, so I'd make this a precondition and replace the first assert with hasArity to make it easier to reason about
Hopefully done, the comment took me a while to parse :)


================
Comment at: clang/lib/Format/UnwrappedLineParser.cpp:4592
+          !Macros.hasArity(ID->TokenText, Args->size())) {
+        // The macro is overloaded to be both object-like and function-like,
+        // but none of the function-like arities match the number of arguments.
----------------
sammccall wrote:
> Strictly I don't think this comment is true, we hit this path when it's **only** an object-like macro, used before parens.
> 
> For this reason you might *not* want the dbgs() here but rather in the bottom `else`
I'm pretty sure we hit this when it's overloaded for both, but we call it with the wrong arity.
E.g. A=x, A(x, y, z)=x y z

A(1, 2)
- Macros.objectLike(A) -> true
- Args -> true
- !Macros.hasArity(A, 2) -> true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144170