[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked an inline comment as done.
ymandel added inline comments.


================
Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:89
+  /// relevant name, not including qualifiers.
+  Name,
+};
----------------
ilya-biryukov wrote:
> ymandel wrote:
> > ilya-biryukov wrote:
> > > Same here, what happens to the template arguments and multi-token names, e.g.
> > > `operator +` or `foo<int, int>`?
> > Good point. This seems difficult to get right, since NamedDecl does not carry sufficient loc data.  However, I've updated the code to explicitly fail in such cases, so at least we won't have bad rewrites.
> > 
> > BTW, any idea whether constructor initializers can ever be multi token?
> > BTW, any idea whether constructor initializers can ever be multi token?
> 
> Two cases come to mind:
> 1. arbitrary names when initializing base classes,  something like `::ns::X<int>(10)`
> 2. template packs with ellipsis (although ellipsis shouldn't be normally part that we replace, I guess): `Base(10)...`
> 
> Full example:
> ```
> namespace ns {
>   struct X {
>     X(int);
>   };
> }
> 
> 
> template <class ...Bases>
> struct Y : ns::X, Bases... {
>   Y() : ns::X(10), Bases(10)... {
>   }
> };
> 
> struct Z {
>   Z(int);
> };
> struct W {
>   W(int);
> };
> 
> Y<Z, W> y;
> ```
Turns out the code was already filtering these cases. I added an addition constrain of I->isWritten() for initializers. So, only explicit initialization of fields is allowed, and therefore I'd venture guaranteed to be a single token. I noticed that Kythe seems to make the same assumption.  

That said, I could change to code to specify the range as a char-range of `getMemberLoc(), getLParenLoc()` if we can't rely on that (single-token) guarantee.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376