[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:53:13
Message-ID: CAKsPscMisFHzk5A22JVCQ6cd3X3qvZtiu6qMX=pwjQ1PjiLWVg () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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">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><div>Steven</div></div><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="HOEnZb"><div class="h5"><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_-3427364582885904685gmail_msg">I do have one more question. <div \
class="m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685gmail_msg"></div><div \
class="m_-3427364582885904685gmail_msg">The return value of add() is an llvm::Error. \
</div><div class="m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685gmail_msg"></div><div \
class="m_-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_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685gmail_msg"></div><div \
class="m_-3427364582885904685gmail_msg">Thanks!</div></div><div class="gmail_extra \
m_-3427364582885904685gmail_msg"><br class="m_-3427364582885904685gmail_msg"><div \
class="gmail_quote m_-3427364582885904685gmail_msg">On Mon, Nov 14, 2016 at 4:06 PM, \
Steven Lu <span dir="ltr" class="m_-3427364582885904685gmail_msg"><<a \
href="mailto:stevenlu443@gmail.com" class="m_-3427364582885904685gmail_msg" \
target="_blank">stevenlu443@gmail.com</a>></span> wrote:<br \
class="m_-3427364582885904685gmail_msg"><blockquote class="gmail_quote \
m_-3427364582885904685gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr" class="m_-3427364582885904685gmail_msg">Hi \
Eric, <div class="m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685gmail_msg"></div><div \
class="m_-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_-3427364582885904685gmail_msg"></div><div \
class="m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685gmail_msg"></div><div \
class="m_-3427364582885904685gmail_msg">It also prompts me to wonder what other use \
cases that this change helps with. </div><div \
class="m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685gmail_msg"></div><div \
class="m_-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_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685gmail_msg"></div><div \
class="m_-3427364582885904685gmail_msg">Thanks</div></div><div \
class="m_-3427364582885904685m_-932104846084891772HOEnZb \
m_-3427364582885904685gmail_msg"><div \
class="m_-3427364582885904685m_-932104846084891772h5 \
m_-3427364582885904685gmail_msg"><div class="gmail_extra \
m_-3427364582885904685gmail_msg"><br class="m_-3427364582885904685gmail_msg"><div \
class="gmail_quote m_-3427364582885904685gmail_msg">On Mon, Nov 14, 2016 at 6:30 AM, \
Eric Liu <span dir="ltr" class="m_-3427364582885904685gmail_msg"><<a \
href="mailto:ioeric@google.com" class="m_-3427364582885904685gmail_msg" \
target="_blank">ioeric@google.com</a>></span> wrote:<br \
class="m_-3427364582885904685gmail_msg"><blockquote class="gmail_quote \
m_-3427364582885904685gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr" class="m_-3427364582885904685gmail_msg">A \
common way is to use the file path in a replacement, i.e. \
`Replaces[R.getFilePath()].<wbr>add(R)`. Note that <a \
href="https://github.com/llvm-mirror/clang/blob/master/include/clang/Tooling/Core/Replacement.h#L146" \
class="m_-3427364582885904685gmail_msg" \
target="_blank">clang::tooling::Replacements</a> is now implemented as a class \
instead of a std::set.<div class="m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685gmail_msg"></div><div \
class="m_-3427364582885904685gmail_msg"><span style="font-size:12.8px" \
class="m_-3427364582885904685gmail_msg">In general, \
`<wbr>formatAndApplyAllReplacements` 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_-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_-3427364582885904685m_-932104846084891772m_-4445786945806028673HOEnZb \
m_-3427364582885904685gmail_msg"><font color="#888888" \
class="m_-3427364582885904685gmail_msg"><div \
class="m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685gmail_msg"></div><div \
class="m_-3427364582885904685gmail_msg">- Eric</div></font></span><div \
class="m_-3427364582885904685gmail_msg"><div \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673h5 \
m_-3427364582885904685gmail_msg"><div class="m_-3427364582885904685gmail_msg"><div \
class="gmail_quote m_-3427364582885904685gmail_msg"><div dir="ltr" \
class="m_-3427364582885904685gmail_msg">On Mon, Nov 14, 2016 at 12:04 PM Manuel \
Klimek <<a href="mailto:klimek@google.com" class="m_-3427364582885904685gmail_msg" \
target="_blank">klimek@google.com</a>> wrote:<br \
class="m_-3427364582885904685gmail_msg"></div><blockquote class="gmail_quote \
m_-3427364582885904685gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg">+eric </div><div dir="ltr" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><div class="gmail_quote \
m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><div dir="ltr" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-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_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg" target="_blank">cfe-dev@lists.llvm.org</a>> \
wrote:<br class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"></div><blockquote class="gmail_quote \
m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><span style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg">I've updated clang from revision 254425 to \
286122, and RefactoringTool::</span><span style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg">getReplacemen<wbr>ts() now produces a map of files \
to replacements. </span><br style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><br style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><span style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-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_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-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_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><br style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><span style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-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_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><div style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"></div><div style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-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_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"><br \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"></div><div style="font-size:12.8px" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg">Thanks</div></div> \
______________________________<wbr>_________________<br \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"> cfe-dev mailing list<br \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"> <a href="mailto:cfe-dev@lists.llvm.org" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg" target="_blank">cfe-dev@lists.llvm.org</a><br \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"> <a \
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg" \
target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br \
class="m_-3427364582885904685m_-932104846084891772m_-4445786945806028673m_93108965231033089gmail_msg \
m_-3427364582885904685gmail_msg"> \
</blockquote></div></div></blockquote></div></div></div></div></div> \
</blockquote></div><br class="m_-3427364582885904685gmail_msg"></div> \
</div></div></blockquote></div><br class="m_-3427364582885904685gmail_msg"></div> \
</blockquote></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