[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-dev
Subject: Re: [cfe-dev] (no subject)
From: Steven Lu via cfe-dev <cfe-dev () lists ! llvm ! org>
Date: 2016-11-16 2:55:26
Message-ID: CAKsPscN27jr0nweY=Km8a9A9hMPbArCDTM=69rpby1ico2w4qg () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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?
On Tue, Nov 15, 2016 at 9:53 PM, Steven Lu <stevenlu443@gmail.com> wrote:
> 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
>
> On Tue, Nov 15, 2016 at 2:06 PM, Eric Liu <ioeric@google.com> wrote:
>
> > You can use "llvm::toString(std::move(Err))" to convert the returned
> > error to string.
> >
> > Thanks for pointing this out :) I'll mention this in the doc.
> >
> > On Tue, Nov 15, 2016 at 7:40 PM Steven Lu <stevenlu443@gmail.com> wrote:
> >
> > > 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!
> > >
> > > On Mon, Nov 14, 2016 at 4:06 PM, Steven Lu <stevenlu443@gmail.com>
> > > wrote:
> > >
> > > 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
> > >
> > > On Mon, Nov 14, 2016 at 6:30 AM, Eric Liu <ioeric@google.com> wrote:
> > >
> > > A common way is to use the file path in a replacement, i.e.
> > > `Replaces[R.getFilePath()].add(R)`. Note that
> > > clang::tooling::Replacements
> > > <https://github.com/llvm-mirror/clang/blob/master/include/clang/Tooling/Core/Replacement.h#L146>
> > > 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
> > > <https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-move/tool/ClangMoveMain.cpp#L137>.
> > > 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 :)
> > >
> > > - Eric
> > > On Mon, Nov 14, 2016 at 12:04 PM Manuel Klimek <klimek@google.com>
> > > wrote:
> > >
> > > +eric
> > >
> > >
> > > On Sat, Nov 12, 2016 at 10:45 AM Steven Lu via cfe-dev <
> > > cfe-dev@lists.llvm.org> wrote:
> > >
> > > 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
> > > _______________________________________________
> > > cfe-dev mailing list
> > > cfe-dev@lists.llvm.org
> > > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> > >
> > >
> > >
> > >
>
[Attachment #5 (text/html)]
<div dir="ltr">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. \
<div><br></div><div>It would indeed be useful to have a way to control this. Is there \
a mechanism for it?</div></div><div class="gmail_extra"><br><div \
class="gmail_quote">On Tue, Nov 15, 2016 at 9:53 PM, Steven Lu <span dir="ltr"><<a \
href="mailto:stevenlu443@gmail.com" \
target="_blank">stevenlu443@gmail.com</a>></span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr">Thanks! <div><br></div><div>Now that I can \
see the errors, they say that my replacements conflict with each other. \
</div><div><br></div><div>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?</div><div><br></div><div>Am I to understand that the replacement subsystem is \
in a state of flux at the moment?</div><div><br></div><div>Thanks a lot!</div><span \
class="HOEnZb"><font color="#888888"><div>Steven</div></font></span></div><div \
class="HOEnZb"><div class="h5"><div class="gmail_extra"><br><div \
class="gmail_quote">On Tue, Nov 15, 2016 at 2:06 PM, Eric Liu <span dir="ltr"><<a \
href="mailto:ioeric@google.com" target="_blank">ioeric@google.com</a>></span> \
wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px \
#ccc solid;padding-left:1ex"><div dir="ltr">You can use \
"llvm::toString(std::move(Err)<wbr>)" to convert the returned error to \
string. <div><br></div><div>Thanks for pointing this out :) I'll mention this in \
the doc.</div></div><div class="m_4368824644463078743HOEnZb"><div \
class="m_4368824644463078743h5"><br><div class="gmail_quote"><div dir="ltr">On Tue, \
Nov 15, 2016 at 7:40 PM Steven Lu <<a href="mailto:stevenlu443@gmail.com" \
target="_blank">stevenlu443@gmail.com</a>> wrote:<br></div><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">I do have one more \
question. <div class="m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">The return value of \
add() is an llvm::Error. </div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">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. </div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">Thanks!</div></div><div \
class="gmail_extra m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><div class="gmail_quote \
m_4368824644463078743m_-3427364582885904685gmail_msg">On Mon, Nov 14, 2016 at 4:06 \
PM, Steven Lu <span dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><<a \
href="mailto:stevenlu443@gmail.com" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg" \
target="_blank">stevenlu443@gmail.com</a>></span> wrote:<br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><blockquote \
class="gmail_quote m_4368824644463078743m_-3427364582885904685gmail_msg" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">Hi Eric, <div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">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.<br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">It also prompts me to \
wonder what other use cases that this change helps with. </div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">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. </div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">Thanks</div></div><div \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772HOEnZb \
m_4368824644463078743m_-3427364582885904685gmail_msg"><div \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772h5 \
m_4368824644463078743m_-3427364582885904685gmail_msg"><div class="gmail_extra \
m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><div class="gmail_quote \
m_4368824644463078743m_-3427364582885904685gmail_msg">On Mon, Nov 14, 2016 at 6:30 \
AM, Eric Liu <span dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><<a \
href="mailto:ioeric@google.com" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg" \
target="_blank">ioeric@google.com</a>></span> wrote:<br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><blockquote \
class="gmail_quote m_4368824644463078743m_-3427364582885904685gmail_msg" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">A common way is to use \
the file path in a replacement, i.e. `Replaces[R.getFilePath()].add<wbr>(R)`. Note \
that <a href="https://github.com/llvm-mirror/clang/blob/master/include/clang/Tooling/Core/Replacement.h#L146" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg" \
target="_blank">clang::tooling::Replacements</a> is now implemented as a class \
instead of a std::set.<div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><span \
style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">In general, \
`formatAndApplyAllReplacements<wbr>` is a function you would use to apply changes \
while format changed code *after* you've got all replacements</span>. A sample \
usage can be found in <a \
href="https://github.com/llvm-mirror/clang-tools-extra/blob/master/clang-move/tool/ClangMoveMain.cpp#L137" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg" \
target="_blank">clang-move</a>. 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 :)</div><span class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673HOEnZb \
m_4368824644463078743m_-3427364582885904685gmail_msg"><font color="#888888" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">- \
Eric</div></font></span><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><div \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673h5 \
m_4368824644463078743m_-3427364582885904685gmail_msg"><div \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"><div class="gmail_quote \
m_4368824644463078743m_-3427364582885904685gmail_msg"><div dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg">On Mon, Nov 14, 2016 at \
12:04 PM Manuel Klimek <<a href="mailto:klimek@google.com" \
class="m_4368824644463078743m_-3427364582885904685gmail_msg" \
target="_blank">klimek@google.com</a>> wrote:<br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div><blockquote \
class="gmail_quote m_4368824644463078743m_-3427364582885904685gmail_msg" \
style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg">+eric </div><div dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><div class="gmail_quote \
m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><div dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg">On Sat, Nov 12, 2016 at 10:45 \
AM Steven Lu via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg" \
target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"></div><blockquote \
class="gmail_quote m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg" style="margin:0 0 0 \
.8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><span style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg">I've updated clang from \
revision 254425 to 286122, and RefactoringTool::</span><span style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg">getReplacemen<wbr>ts() now \
produces a map of files to replacements. </span><br style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><br style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><span style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg">My original code implements a \
MatchFinder::MatchCallback which takes a pointer to the return value of \
RefactoringTool::</span><span style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg">getReplacemen<wbr>ts() 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. </span><br style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><br style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><span style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg">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. </span><br style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><div style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg">I wonder if someone could do a \
quick rundown of how to use the interface in a proper way. </div><div \
style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"><br \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"></div><div \
style="font-size:12.8px" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg">Thanks</div></div> \
______________________________<wbr>_________________<br \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"> cfe-dev mailing list<br \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"> <a \
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg" \
target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br \
class="m_4368824644463078743m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_4368824644463078743m_-3427364582885904685gmail_msg"> \
</blockquote></div></div></blockquote></div></div></div></div></div> \
</blockquote></div><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div> \
</div></div></blockquote></div><br \
class="m_4368824644463078743m_-3427364582885904685gmail_msg"></div> \
</blockquote></div> </div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
[Attachment #6 (text/plain)]
_______________________________________________
cfe-dev mailing list
cfe-dev@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic