[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

Manuel Klimek via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 4 05:09:27 PDT 2017


klimek added inline comments.


================
Comment at: test/Refactor/LocalRename/Field.cpp:4
+class Baz {
+  int /*range=*/Foo; // CHECK: symbol [[@LINE]]:17 -> [[@LINE]]:20
+public:
----------------
arphaman wrote:
> klimek wrote:
> > arphaman wrote:
> > > klimek wrote:
> > > > Does this just test the selection?
> > > No, this is the moved `clang-rename/Field.cpp` test that tests local-rename. I will move the other tests when this patch is accepted.
> > Ok, then I find this test really hard to read - where does it check what the symbol was replaced with?
> I thought I should check for occurrences, but you're right, it's probably better to always check the source replacements (we can introduce options that apply non-default occurrences in the future which would allow us to check replacements for occurrences in comments). 
> 
> Would something like `//CHECK: "newName" [[@LINE]]:24 -> [[]]:27` work when checking for replacements?
Yep, I think that'd be better.
Or we could just apply the replacement and check on the replaced code? (I think that's what we do in fixits for clang and clang-tidy)


Repository:
  rL LLVM

https://reviews.llvm.org/D36574





More information about the cfe-commits mailing list