[PATCH] D99561: Support visitor pattern by PointerUnion.

Scott Linder via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Apr 5 22:39:46 PDT 2021


scott.linder added a comment.

To chime in on the spelling of things relative to `std`, I had similar questions for IntrusiveVariant in https://reviews.llvm.org/D98477

A natural question that might help with deciding how to handle `visit` is: how should we spell alternative type access? `std` uses the free function `std::get`. It makes sense, as that same free function is overloaded for e.g. `std::tuple`, `std::pair`, `std::variant`, etc. but we don't currently have anything similar in ADT.

I think my leaning would be to go all-in on the `std` approach by doing the following:

- Add an `llvm::adt` namespace (or something similar) containing things which are "designed to fulfill the same roles as the identically-named member of the `std` namespace, but for whatever reason we cannot actually use that member directly". Generally the reason will be: that thing is not an extension point, and it is forbidden to re-open `std` to overload that thing.
- Add the appropriate `llvm::adt::get(llvm::PointerUnion)` overloads
- When adding `InrusiveVariant`, I can add `llvm::adt::get(llvm::IntrusiveVariant)` overloads
- Add `llvm::adt::visit` and the appropriate `llvm::PointerUnion` overloads
- When adding `IntrusiveVariant`, I can add `llvm::adt::visit` overloads

Then everything is spelled the same/quacks the same, but names are not ambiguous in the context of `using namespace llvm;` or `using namespace std;` or both.

For other things, where we expect to delete our copy entirely once we move the codebase to a certain `std` version, we can just drop the exact "polyfill" definition with the same name into `llvm::`, and then delete the code later. An example I plan to post a patch for is `in_place{_type,_index}_t`.

What are people's thoughts?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99561



More information about the llvm-commits mailing list