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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits