<div>Thanks for the input!</div><div><br></div><div>Tooling/Refactoring is definitely the right way to go - dumping to stdout was just a holdover from the example on LibTooling. I'll change it once I figure out how it works - and a clean way to arrange the tests.</div>
<div><br></div><div>As for the use of matchers vs. visitors, I decided to use a visitor because this is a relatively complex transformation. I would happily use matchers if I thought I could - and I think that some other c++11 migrations can easily be written with matchers - but I think the for loop checks need some features that matchers don't have (correct me if I'm wrong!). For example, the check for statically allocated array-based loops does this:</div>
<div>Given a for loop, determine if:<br><div> - The increment statement increments (via ++) exactly one integral index variable, such that the variable is declared and initialized to zero in the init portion of the loop, and that this variable's value is a compared (via <, > or !=) to a nonnegative compile-time constant N in the compare portion.</div>
<div> - The index variable is only ever used in an ArrayIndexExpession indexing a single, statically allocated array A.</div><div> - The array A has exactly N elements.</div><div> - Additionally, if the ArrayIndexExpression A[index] is ever assigned, passed to a function or copied as a non-const reference, or its address taken with & (I still need to add a check for calls to non-const member functions), the loop variable in the converted version needs to be a non-const reference so that the value will be correctly updated (this step adds the most complexity).</div>
</div><div><br></div><div>The other types of loop (iterator-based, and array-like container) are more complicated to detect, since there are more permitted ways to define and use the index/iterators. What makes this difficult to do entirely with matchers is the number of back- and cross-references, as well as the different local behaviors based on semantic properties. On the other hand, if there were some kind of backreference-enabled matcher that allowed me to locate all matches in a given Stmt, it could be *much* easier to express some parts of the logic, such as the first step in the above list. I also suspect that a single-Stmt matcher would better way to handle the last step; currently I track whether the visitor is looking at a statement or expression which fits any of the const-disqualifying conditions, and a note is made if I run into A[index].</div>
<div><br></div><div>Does this make the use case clearer? I don't really see a better way to approach this problem, but you guys know the available toolkit far better than I do.</div><div class="gmail_extra"><br><div class="gmail_quote">
On Fri, Jun 29, 2012 at 2:48 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@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 class="gmail_quote"><div class="im">On Fri, Jun 29, 2012 at 11:45 AM, Chandler Carruth <span dir="ltr"><<a href="mailto:chandlerc@google.com" target="_blank">chandlerc@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
I tend to agree that this should use the Tooling/Refactoring stuff.<div><br></div><div>However, I'm curious about the best way to structure the location of migration candidates: AST matchers vs. visitor.</div><div><br>
</div><div>I can almost see the visitor pattern working really well here as each different construct can have a pile of migration logic dropped in.... But if there is a need to connect dots between more distant constructs, that wouldn't work so well.... Not at all sure what would be best here.</div>
</blockquote><div><br></div></div><div>I've used a combination before - use matchers for the stuff where they work well, then write a very small easy-to-understand visitor if you need more. I think that brings down code size by quite a bit - obviously just a gut feeling here.</div>
<div><div class="h5">
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Jun 29, 2012 at 1:37 AM, Manuel Klimek <span dir="ltr"><<a href="mailto:klimek@google.com" target="_blank">klimek@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 class="gmail_quote"><div>On Fri, Jun 29, 2012 at 4:06 AM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
In case anyone wanted to take a look, the attached patch includes the tool I've been working on. I create a new binary, c++migrate, which attempts to convert for loops in the source files given to it. Most of my focus has been on the FrontedAction, so I skirted all of the issues mentioned above by keeping the frontend interaction minimal (i.e. I just call Tooling::ClangTool::run), and the changes are just reported on standard output, if there are any to be made.<div>
<br></div><div>The tool can currently convert for loops that range over (1) statically allocated arrays, and (2) Clang-style iterator-based loops (with begin and end iterators defined). All loop variables need to be declared within the loop's initialization step in order for it to be converted, though this requirement can potentially be eliminated. I'm working on converting iterator-based loops that call someContainer.end() on each iteration, since they're probably the common case in many codebases.</div>
<div><br></div><div>Just for fun, I ran the tool over the 41 .cpp files in lib/Sema, and my tool found 71 convertible loops in 17 files. There is plenty more work to go, because it clearly missed some easy ones.</div><div>
<br></div><div>Any input or feedback is welcome!</div></blockquote><div><br></div></div><div>High-level observations:</div><div>1. the handling of the rewrites; any reason not to use the Tooling/Refactoring stuff? Currently in the patch it looks to me like the files are not rewritten, but dumped to stdout</div>
<div>2. is the reason not to use the matchers here that they're not landed in mainline yet?</div><div><br></div><div>Cheers,</div><div>/Manuel</div><div><div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><font color="#888888"><div><br></div><div>-Sam</div></font></span><div><div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jun 28, 2012 at 10:50 AM, Sam Panzer <span dir="ltr"><<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I'm that intern :)<span><font color="#888888"><div><br></div><div>-Sam</div></font></span><div>
<div><div class="gmail_extra"><br><br><div class="gmail_quote">On Wed, Jun 27, 2012 at 9:48 PM, John Wiegley <span dir="ltr"><<a href="mailto:johnw@boostpro.com" target="_blank">johnw@boostpro.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div>>>>>> Sam Panzer <<a href="mailto:panzer@google.com" target="_blank">panzer@google.com</a>> writes:<br>
<br>
> In particular, I am working on a tool to convert existing C++ for loops to<br>
> take advantage of the new C++11 range-based syntax. I can imagine similar<br>
> tools to replace const with constexpr, macro hacks with static_assert, and<br>
> potentially other common refactorings.<br>
<br>
> Thoughts? Suggestions?<br>
<br>
</div>You really must watch this presentation, if you haven't already:<br>
<br>
<a href="http://www.youtube.com/watch?v=yuIOGfcOH0k" target="_blank">http://www.youtube.com/watch?v=yuIOGfcOH0k</a><br>
<span><font color="#888888"><br>
--<br>
John Wiegley<br>
BoostPro Computing<br>
<a href="http://www.boostpro.com" target="_blank">http://www.boostpro.com</a><br>
_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div></div></div><br>
<br>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div></div></div><br>
</blockquote></div><br></div>