[PATCH] D139779: [Support] Add transformOptional

Kazu Hirata via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Dec 10 17:56:22 PST 2022


kazu added a comment.

In D139779#3986656 <https://reviews.llvm.org/D139779#3986656>, @dblaikie wrote:

> Could we call it `transform`, or does that complicate things with overload resolution/other functions named `transform`?

The compiler and overload resolutions are OK.  With the function signature being very narrowly scoped, the proposed function should only match the intended use.  (In fact, I used to call this function `transform` while working on the patch.)

I am not sure if people are OK.  `clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:221` uses both `llvm::transform` (defined in `llvm/include/llvm/ADT/STLExtras.h`) and `llvm::Optional<T>::transform`.  Having two instances of `llvm::transform` in one statement is pretty confusing.

But then I personally find the name of `transformOptional` a bit ugly, which you may also be implying, so I am sitting on the fence as far as the naming goes.  You get to break the tie for me. :-)

I'll look into sharing `MoveOnly`.  Maybe I can come up with a preparation patch to share that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139779



More information about the llvm-commits mailing list