This function has been implemented in several places, so I figure it might be useful to have this function in the library. Although I'm not sure whether this should be a class interface or just a standalone helper function. <div><br></div><div>@alexshap: thanks for the explanation. For your point B, I'm not sure about the example, could you elaborate a bit? E.g. which use case to be specific? What are the headers?</div><div><br></div><div>Generally, the way users of `Replacements` class adding new replacements also matters. For example, if you have multiple insertions at the same offset, you could concatenate them and insert as one single replacement, if you care about performance. </div><div><br></div><div>For most use cases I've seen when converting from old std::set implementation to the current implementation, performance can be improved by changing the way replacements are added. And the most natural way is often the most efficient one.<br><br><div class="gmail_quote"><div dir="ltr">On Sat, Sep 10, 2016, 10:54 Alexander Shaposhnikov <<a href="mailto:shal1t712@gmail.com">shal1t712@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">alexshap added a comment.<br class="gmail_msg">
<br class="gmail_msg">
disclaimer: i don't know if this method is the right thing to add (to be honest i would prefer a better interface but don't have any good suggestions on my mind at the moment), i see several issues (IMO, i might be mistaken, apologize in advance) with the current interface of Replacements. I will not list all of them, but want to add some context:<br class="gmail_msg">
<br class="gmail_msg">
A. right now i see the same code (with minor changes) in several places:<br class="gmail_msg">
<br class="gmail_msg">
1.<br class="gmail_msg">
<br class="gmail_msg">
llvm/tools/clang/tools/extra/include-fixer/IncludeFixer.cpp +382<br class="gmail_msg">
<br class="gmail_msg">
auto R = tooling::Replacement(<br class="gmail_msg">
{FilePath, Info.Range.getOffset(), Info.Range.getLength(),<br class="gmail_msg">
Context.getHeaderInfos().front().QualifiedName});<br class="gmail_msg">
auto Err = Replaces.add(R);<br class="gmail_msg">
if (Err) {<br class="gmail_msg">
llvm::consumeError(std::move(Err));<br class="gmail_msg">
R = tooling::Replacement(<br class="gmail_msg">
R.getFilePath(), Replaces.getShiftedCodePosition(R.getOffset()),<br class="gmail_msg">
R.getLength(), R.getReplacementText());<br class="gmail_msg">
Replaces = Replaces.merge(tooling::Replacements(R));<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
2.<br class="gmail_msg">
<br class="gmail_msg">
llvm/tools/clang/tools/extra/change-namespace/ChangeNamespace.cpp +126 (see <a href="https://reviews.llvm.org/D24183" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D24183</a>)<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
void addOrMergeReplacement(const tooling::Replacement &R,<br class="gmail_msg">
tooling::Replacements *Replaces) {<br class="gmail_msg">
auto Err = Replaces->add(R);<br class="gmail_msg">
if (Err) {<br class="gmail_msg">
llvm::consumeError(std::move(Err));<br class="gmail_msg">
auto Replace = getReplacementInChangedCode(*Replaces, R);<br class="gmail_msg">
*Replaces = Replaces->merge(tooling::Replacements(Replace));<br class="gmail_msg">
}<br class="gmail_msg">
}<br class="gmail_msg">
<br class="gmail_msg">
B. For replacements in headers we can get into if(Err) branch quite often because the same replacement can be generated multiple times (if that header is included into several *.cpp files).<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<a href="https://reviews.llvm.org/D24383" rel="noreferrer" class="gmail_msg" target="_blank">https://reviews.llvm.org/D24383</a><br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
<br class="gmail_msg">
</blockquote></div></div>