[PATCH] D61467: [Rewrite] Extend to further accept CharSourceRange

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.


================
Comment at: cfe/trunk/unittests/Rewrite/CMakeLists.txt:12
   clangRewrite
+  clangTooling
   )
----------------
jdenny wrote:
> thakis wrote:
> > This makes RewriteTests depend on clangTooling, and in follow-ups on clangFrontend and clangSerialization. Rewrite used to depend on basically only clangBasic and clangLex, and now it depends on almost all of clang. Maybe there's a less heavy-weight way to test this?
> > 
> > Also, when do you expect to land code that uses this? Checking in code that's unused upstream over a long period of time seems suboptimal. I delete code that shows up unused on http://llvm-cs.pcc.me.uk/ every now and then for example. (Ignore this part if you expect to check in clients of the overload soon.)
> > This makes RewriteTests depend on clangTooling, and in follow-ups on clangFrontend and clangSerialization. Rewrite used to depend on basically only clangBasic and clangLex, and now it depends on almost all of clang. Maybe there's a less heavy-weight way to test this?
> 
> Sure, I can look for another way to test this.  I didn't realize this would be a real problem.  But....
> 
> > Also, when do you expect to land code that uses this?
> 
> Not soon.  I offered this patch thinking that obvious Rewrite extensions like this one could prove useful to the larger community even if there are no upstream users now.  I'm also fine to revert and keep my Rewrite extensions private for now.  Is that the best approach?
>   This makes RewriteTests depend on clangTooling, and in follow-ups on clangFrontend and clangSerialization. Rewrite used to depend on basically only clangBasic and clangLex, and now it depends on almost all of clang. Maybe there's a less heavy-weight way to test this?

I'm thinking of adding more Rewrite unit tests in the future.  Does anyone have advice on this issue?  That is, is there any easy way to build ASTs in unit tests without such dependencies?  On the other hand, these dependencies are common throughout `clang/unittests` subdirectories, so I'm not sure why RewriteTests needs to avoid them.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61467