(no subject)

I’ve updated clang from revision 254425 to 286122, and RefactoringTool::getReplacements() now produces a map of files to replacements.

My original code implements a MatchFinder::MatchCallback which takes a pointer to the return value of RefactoringTool::getReplacements() and simply inserts Replacements, now it is not clear how I am to perform the insertion, I cannot tell now if there is some canonical way to determine the filename string to save a particular Replacement under.

I have also found the function formatAndApplyAllReplacements which I’m having trouble finding doc/instructions for and seems to be a highest-level function that I should use, but it doesnt seem to help address this problem.

I wonder if someone could do a quick rundown of how to use the interface in a proper way.

Thanks

+eric

A common way is to use the file path in a replacement, i.e. Replaces[R.getFilePath()].add(R). Note that clang::tooling::Replacements is now implemented as a class instead of a std::set.

In general, formatAndApplyAllReplacements is a function you would use to apply changes while format changed code after you’ve got all replacements. A sample usage can be found in clang-move. And I think the interface’s comment does explain its purpose. Which specific part did you find confusing? I’m happy to improve the doc :slight_smile:

  • Eric

Hi Eric,

Thanks for clarifying. The only confusion I have is that previously the API was easier to use. Now I have to call Replacement.getFilePath() and explicitly index the filename map. It feels almost like maybe instead of getReplacements() returning a std::map maybe this could be (yet another) class so we can continue to have this straightforward interface.

It also prompts me to wonder what other use cases that this change helps with.

As for doc I’m sure that just adding a note of what you said (use Replacement.getFilePath() to index to the map) would have been enough to get me there. I hadn’t realized that Replacement could fetch the file path.

Thanks

I do have one more question.

The return value of add() is an llvm::Error.

I’m now finally able to compile the code, but I’m having a ton of trouble figuring out how to print such an error to a string stream of any kind. Google is no help here.

Thanks!

You can use “llvm::toString(std::move(Err))” to convert the returned error to string.

Thanks for pointing this out :slight_smile: I’ll mention this in the doc.

Thanks!

Now that I can see the errors, they say that my replacements conflict with each other.

It seems to me that if I make multiple replacements placed at the same location, which all replace zero characters, this shouldn’t be treated as a conflict. This is in fact what I am seeing. Could you comment on that?

Am I to understand that the replacement subsystem is in a state of flux at the moment?

Thanks a lot!
Steven

I wonder if the logic behind the conflict is that multiple replacements (which are effectively additions as they remove zero characters) placed in the same location makes it unclear which order to apply them in.

It would indeed be useful to have a way to control this. Is there a mechanism for it?

Please read the documentation of Replacements::add() interface for definition of conflict and possible ways to to deal with such conflict.

For your use case specifically, here is a way to handle this: https://github.com/llvm-mirror/clang-tools-extra/blob/master/include-fixer/IncludeFixer.cpp#L385

And you are right, we do need a way to control the order of insertions, but tooling::Replacements is not the right level of abstraction here. I am work on a structure that is one layer higher and will come with an interface for this.