alexshap added inline comments.
================
Comment at: cfe/trunk/lib/Tooling/Core/Replacement.cpp:182
@@ +181,3 @@
+llvm::Expected<Replacements>
+Replacements::mergeIfOrderIndependent(const Replacement &R) const {
+ Replacements Rs(R);
----------------
ioeric wrote:
> alexshap wrote:
> > sorry, probably i am late to the party,
> > however i'd like to ask how this method is supposed to be used by the
> > customers of this code.
> > If i'm not mistaken we have some concrete examples in clang-extra-tools:
> > for instance @omtcyfz referred to this diff on
> > https://reviews.llvm.org/D24914.
> > Basically after this diff the class Replacements contains several methods
> > (add, mergeIfOrderIndependent etc), but mergeIfOrderIndependent potentially
> > will copy a lot of replacements (i don't know if the perf is a real issue
> > here, but at least it looks a bit strange to me).
> > Another question - how are the customers of this code supposed to use the
> > method add ? still suppress the error (like what was happening on
> > https://reviews.llvm.org/D24914) ?
> > Am i missing smth ?
> > cc: @klimek, @djasper.
> >
> First of all, this diff doesn't introduce new interfaces - they are just
> private helper functions, which are mostly called on conflicting replacements
> set with relatively small size.
>
> Secondly, `add` now supports merging order-independent replacements, so it
> handles deduplication to some extend, i.e. identical non-insertion
> replacements can be deduplicated, which happens to solve the issue with
> clang-rename as mentioned in D24914.
>
i see, thank you for the explanation.
Repository:
rL LLVM
https://reviews.llvm.org/D24800
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
