[PATCH] D153152: Adds tweak to add declarations for pure virtuals

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 25 13:29:47 PDT 2023


sammccall added a comment.

In D153152#4617391 <https://reviews.llvm.org/D153152#4617391>, @robot wrote:

>> so you could replace the logic that prints the return type + arg list with `printType(functype, class, methodname)` (from AST.h). That should pick up noexcept etc.
>
> I've tried it and it's not obvious to me how to do it properly: Since `printType` wants to print the _function type_, it will parenthesize the placeholder (methodname):
>
>   void (foo)()
>
> I've tried adding a flag to `PrintingPolicy` to avoid parenthesizing the name, but I wonder if adjusting a "type printer" to print a function declaration is a clean design. Next problem I ran into is parameters: A type printer does not need to concern itself with parameter names nor default arguments:
>
>   void foo(int x = 0) = 0;
>   void foo(int) override;
>
> Shall I continue to pursue that path? Shall I adjust the type printer to be able to print parameter names and default arguments?

Sorry, it does sound like I sent you down a bad path.
You *could* (no need for an option for parens, here we can detect that the placeholder isValidAsciiIdentifier), but entirely possible more things are broken still.

If you prefer the approach with copying the tokens, adding heuristics to patch them up as needed is maybe no worse than the alternatives.

(BTW, I don't think we want to copy default arguments - it's certainly not necessary, and not helpful in common cases where the function is only called through the base)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153152



More information about the cfe-commits mailing list