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

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

jdenny added inline comments.


================
Comment at: cfe/trunk/unittests/Rewrite/CMakeLists.txt:12
   clangRewrite
+  clangTooling
   )
----------------
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?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61467