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

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.


================
Comment at: clang/unittests/Format/FormatTest.cpp:22584
+  Style.Macros.push_back("CALL(x)=f([] { x })");
+  Style.Macros.push_back("ASSIGN_OR_RETURN(a, b, c)=a = (b) || (c)");
+
----------------
sammccall wrote:
> klimek wrote:
> > sammccall wrote:
> > > If this is assign_or_return, the treatment of "c" is fairly strange. Also you are mostly calling this with only two args. Maybe drop C and use a different macro for the 3-arg case?
> > ASSIGN_OR_RETURN allows 3 args, though; this is basically the thing I'd propose to configure for ASSIGN_OR_RETURN in general; is there anything specific you don't like about this?
> OK, this is a bit sticky.
>  - `ASSIGN_OR_RETURN` is almost always called with two args
>  - some versions of `ASSIGN_OR_RETURN` support an optional third arg (there's no canonical public version)
>  - these emulate overloading using variadic macro tricks
>  - this patch doesn't claim to support either variadic macros or overloading
> 
> So the principled options seem to be:
>  - macros have fixed arity: clang-format can support the two-arg version of `ASSIGN_OR_RETURN` but not the three-arg version. (This is what I was assuming)
>  - macros have variable arity: the public docs need to describe how passing too many/too few args is treated, because this isn't obvious and it sounds like we want people to rely on it. This feels like stretching "principled" to me - we're relying on the expansion still parsing correctly when args are missing, which basically seems like coincidence.
>  - macros support overloading: instead of looking up macros by name, the key is `(string Name, optional<int> Arity)`
Chose option 3, as this is pretty straight-forward and I believe the best user experience.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144170