[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-14 21:06:11
Message-ID: CAKsPscMdrbGwZQw98a8YYY_RrwxoXNf6Q70ExHVaFY081O4pFw () mail ! gmail ! com
[Download RAW message or body]
[Attachment #2 (multipart/alternative)]
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">Hi Eric, <div><br></div><div>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></div><div><br></div><div>It also prompts me to wonder what other use \
cases that this change helps with. </div><div><br></div><div>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><br></div><div>Thanks</div></div><div class="gmail_extra"><br><div \
class="gmail_quote">On Mon, Nov 14, 2016 at 6:30 AM, 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">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" \
target="_blank">clang::tooling::Replacements</a> is now implemented as a class \
instead of a std::set.<div><br></div><div><span style="font-size:12.8px">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" \
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="HOEnZb"><font color="#888888"><div><br></div><div>- \
Eric</div></font></span><div><div class="h5"><div><div class="gmail_quote"><div \
dir="ltr">On Mon, Nov 14, 2016 at 12:04 PM Manuel Klimek <<a \
href="mailto:klimek@google.com" target="_blank">klimek@google.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_93108965231033089gmail_msg">+eric </div><div dir="ltr" \
class="m_93108965231033089gmail_msg"><br class="m_93108965231033089gmail_msg"><br \
class="m_93108965231033089gmail_msg"><div class="gmail_quote \
m_93108965231033089gmail_msg"><div dir="ltr" class="m_93108965231033089gmail_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_93108965231033089gmail_msg" \
target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br \
class="m_93108965231033089gmail_msg"></div><blockquote class="gmail_quote \
m_93108965231033089gmail_msg" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex"><div dir="ltr" class="m_93108965231033089gmail_msg"><span \
style="font-size:12.8px" class="m_93108965231033089gmail_msg">I've updated clang \
from revision 254425 to 286122, and RefactoringTool::</span><span \
style="font-size:12.8px" class="m_93108965231033089gmail_msg">getReplacemen<wbr>ts() \
now produces a map of files to replacements. </span><br style="font-size:12.8px" \
class="m_93108965231033089gmail_msg"><br style="font-size:12.8px" \
class="m_93108965231033089gmail_msg"><span style="font-size:12.8px" \
class="m_93108965231033089gmail_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_93108965231033089gmail_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_93108965231033089gmail_msg"><br style="font-size:12.8px" \
class="m_93108965231033089gmail_msg"><span style="font-size:12.8px" \
class="m_93108965231033089gmail_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_93108965231033089gmail_msg"><div style="font-size:12.8px" \
class="m_93108965231033089gmail_msg"><br \
class="m_93108965231033089gmail_msg"></div><div style="font-size:12.8px" \
class="m_93108965231033089gmail_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_93108965231033089gmail_msg"><br \
class="m_93108965231033089gmail_msg"></div><div style="font-size:12.8px" \
class="m_93108965231033089gmail_msg">Thanks</div></div> \
______________________________<wbr>_________________<br \
class="m_93108965231033089gmail_msg"> cfe-dev mailing list<br \
class="m_93108965231033089gmail_msg"> <a href="mailto:cfe-dev@lists.llvm.org" \
class="m_93108965231033089gmail_msg" target="_blank">cfe-dev@lists.llvm.org</a><br \
class="m_93108965231033089gmail_msg"> <a \
href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev" rel="noreferrer" \
class="m_93108965231033089gmail_msg" \
target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-dev</a><br \
class="m_93108965231033089gmail_msg"> \
</blockquote></div></div></blockquote></div></div></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