[PATCH] D86169: Initial support for letting plugins perform custom parsing of attribute arguments.

[Jonathan Protzenko via Phabricator via cfe-commits]

jonathan.protzenko added a comment.

Thanks for the review!

Regarding Declarator: I briefly mentioned something related in the details section... actually maybe you can tell me what's the best approach here. I was thinking of just passing in a void* and expecting plugins to do

  if (const Declarator *D = isa_and_nonnull<Declarator>(Context))

This would allow a future-proof API where we can, in the future, pass in more classes for the "Context" argument (not just a Declarator), relying on clients doing dynamic casts. If I understood https://llvm.org/docs/ProgrammersManual.html#isa this `void*` cast would work?

My only hesitation is I'm not sure about the right technical device to use for this polymorphism pattern, as I've seen a couple related things in the clang code base:

- I've seen a pattern where there's a wrapper class that has a Kind() method and then allows downcasting based on the results of the Kind (seems overkill here)
- I've also seen references to `opaque_ptr` somewhere with a `get<T>` method... (maybe these are helpers to do what I'm doing better?)

If you can confirm the void* is the right way to go (or point me to a suitable pattern I can take inspiration from), I'll submit a revised patch, along with the extra spelling argument (good point, thanks!).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86169