<div class="gmail_extra"><div class="gmail_quote">On Thu, Jul 12, 2012 at 11:07 PM, 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="im"><br>
On Thu, Jul 12, 2012 at 11:59 PM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>> wrote:<br>
> I accidentally sent the email before I was done (the Gmail tab/space combo<br>
> strikes again), and you replied before I finished fixing the original :)<br>
><br>
> On Thu, Jul 12, 2012 at 2:37 PM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
>><br>
>> On Thu, Jul 12, 2012 at 11:31 PM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>> wrote:<br>
>>><br>
>>> Here are three kinds of operations that I would want to completely avoid<br>
>>> any kind of recursive AST traversal:<br>
>>>  - Optional matchers: In order to ignore certain ignorable parts of the<br>
>>> AST, I often want to create<br>
>><br>
>><br>
>> Is there something missing?<br>
><br>
><br>
> The missing part of optional matchers: I'm just wondering if a shorthand<br>
> would be reasonable to implement. instead of something like<br>
>   anyOf(unaryOperator(hasOperand(SomeOtherMatcher)), SomeOtherMatcher)<br>
> One could use<br>
>   optional(unaryOperator(hasOperand), SomeOtherMatcher)<br>
<br>
</div>And we arrive at where the problem is with something that looks like a<br>
functional language but doesn't have a really nice way to express<br>
higher order functions without lots of syntactical crap around it.<br>
<br>
But it also doesn't strike me as so bad to repeat SomeOtherMatcher in there...<br></blockquote><div><br></div><div>I was just curious if this was possible, though I didn't expect it to be. It would only have been really useful when combined with the recursion request.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="im"><br>
>>>  - Right-recursive matchers:<br>
>>><br>
>>>   I have a function that hunts for the expression which is actually<br>
>>> passed to a single-parameter constructor, which sheds ImplicitCastExprs,<br>
>>> single-parameter CXXConstructExprs, and MaterializeTemporaryExprs until it<br>
>>> finds something that is not a single-parameter constructor. Essentially, the<br>
>>> matcher would look like<br>
>>>   StmtMatcher =<br>
>>> expression(rightRecursion(ignoreImplicitCasts(constructorCall(argumentCountIs(1),<br>
>>> hasArgument(0, optional(materializeTemporaryExpr(RecursionPoint))),<br>
>>> withBaseCase(id(expresion))))<br>
>><br>
>><br>
>> hasDescendant / forEachDescendant doesn't work?<br>
><br>
><br>
> I am trying to enforce a particular structure on the way to those<br>
> descendants - I only want to match the innermost expression if the AST was a<br>
> linear chain of these particular kinds of node, and {has,forEach}Descendant<br>
> throws this information away. If I'm wrong, that would be great!<br>
<br>
</div>hasDescendant will match at the first occurrence in tree-search order<br>
(RecursiveASTVisitor order)<br>
<br></blockquote><div><br></div><div>I think the difficulty here is matching against a recurring pattern. In some sense, what I'm really looking for is the first descendant that doesn't match. To make this as explicit as possible, my current code looks like this:</div>
<div><br></div><div><div>const Expr *digThroughConstructors(const Expr *E) {</div><div>  if (!E)</div><div>    return NULL;</div><div>  E = E->IgnoreParenCasts();</div><div>  if (const CXXConstructExpr *ConstructExpr = dyn_cast<CXXConstructExpr>(E)) {</div>
<div>    if (ConstructExpr->getNumArgs() != 1)</div><div>      return NULL;</div><div>    E = ConstructExpr->getArg(0);</div><div>    if (const MaterializeTemporaryExpr *MTE = dyn_cast<MaterializeTemporaryExpr>(E))</div>
<div>      E = MTE->GetTemporaryExpr();</div><div>    return digThroughConstructors(E);</div><div>  }</div><div>  return E;</div><div>}</div></div><div><br></div><div>It's short enough that I don't mind leaving it as its own separate check. Thanks again for taking the time to help me with this!</div>
<div><br></div><div>-Sam</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Cheers,<br>
/Manuel<br>
<div class=""><div class="h5"><br>
>>>  - The ability to run the equivalent of a MatchFinder on an arbitrary AST<br>
>>> (e.g. in the callback of another matcher)<br>
>><br>
>><br>
>> That shouldn't be too hard to implement.<br>
><br>
><br>
> Excellent!<br>
><br>
> Thanks,<br>
> -Sam<br>
><br>
>><br>
>><br>
>>><br>
>>><br>
>>><br>
>>> On Thu, Jul 12, 2012 at 1:06 AM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>> wrote:<br>
>>>><br>
>>>> On Wed, Jul 11, 2012 at 11:17 PM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>> wrote:<br>
>>>> ><br>
>>>> ><br>
>>>> ><br>
>>>> > On Wed, Jul 11, 2012 at 11:20 AM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>>>> > wrote:<br>
>>>> >><br>
>>>> >> On Wed, Jul 11, 2012 at 8:08 PM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>><br>
>>>> >> wrote:<br>
>>>> >> > Good point. I currently have a matcher that finds for loops that<br>
>>>> >> > appear<br>
>>>> >> > to<br>
>>>> >> > have the right shape for conversion, and I do some extra checking<br>
>>>> >> > along<br>
>>>> >> > with<br>
>>>> >> > the conversion work in the callback. I can completely handle<br>
>>>> >> > array-style<br>
>>>> >> > loops with a Preprocessor, which is used when generating the names<br>
>>>> >> > of<br>
>>>> >> > new<br>
>>>> >><br>
>>>> >> I don't understand yet what the Preprocessor is needed for here...<br>
>>>> ><br>
>>>> ><br>
>>>> > I use Preprocessor in exactly one place right now, namely to create an<br>
>>>> > IdentifierResolver. When I am generating the name of a new loop<br>
>>>> > variable, I<br>
>>>> > want to make sure that the identifier is unique, which is an<br>
>>>> > approximation<br>
>>>> > to checking that the identifier neither shadows an existing definition<br>
>>>> > nor<br>
>>>> > is shadowed in an inner scope. If there's a better way to do this, I<br>
>>>> > would<br>
>>>> > be happy to change it.<br>
>>>><br>
>>>> The ASTContext has the identifier table. (ASTContext::Idents).<br>
>>>><br>
>>>> >> > variables. For iterator-style loops, I use Sema to determine the<br>
>>>> >> > return<br>
>>>> >> > type<br>
>>>> >> > of operator*, which will be the type of the loop variable. Finally<br>
>>>> >> > (though<br>
>>>> >><br>
>>>> >> Isn't the return type of the oeprator* already in the AST?<br>
>>>> ><br>
>>>> ><br>
>>>> > It's actually possible that operator* is never called. My initial<br>
>>>> > implementation tried to infer the type from  expressions involving the<br>
>>>> > iterator, but this didn't work correctly for CXXMemberCallExpr. I can<br>
>>>> > conceivably work around this without checking operator*, but<br>
>>>><br>
>>>> did you want to finish that<br>
>>>><br>
>>>> >> > this isn't implemented yet), containers that are used as if they're<br>
>>>> >> > arrays<br>
>>>> >> > will require Sema to check if suitable begin() and end() functions<br>
>>>> >> > exist.<br>
>>>> >><br>
>>>> >> Ah, I see - basically you want to trigger overload resolution to see<br>
>>>> >> whether the conversion would work. That makes sense. I'm not sure we<br>
>>>> >> want to get all of Sema available though - it's an insanely huge<br>
>>>> >> interface, and it's rather hard to figure out what's safe to do and<br>
>>>> >> what's not.<br>
>>>> >><br>
>>>> >> Perhaps we can create a class that encapsulates Sema for those<br>
>>>> >> "what-if" type questions we'll need to answer for refactoring tools?<br>
>>>> ><br>
>>>> ><br>
>>>> > This sounds like a good idea. I can try to identify exactly the<br>
>>>> > features I<br>
>>>> > need (most likely just overload resolution).<br>
>>>><br>
>>>> That would sound like most of what we need. It has come up a few<br>
>>>> times. Let me know if you identify more things we need.<br>
>>>><br>
>>>> Cheers,<br>
>>>> /Manuel<br>
>>>><br>
>>>> ><br>
>>>> >><br>
>>>> >><br>
>>>> >> > For now, I can force the use of auto rather than an explicit type<br>
>>>> >> > listing<br>
>>>> >> > for iterator-based loops and get most of the work done without<br>
>>>> >> > Sema. The<br>
>>>> >> > question then becomes the correct way to avoid repeatedly creating<br>
>>>> >> > a<br>
>>>> >> > Preprocessor object, though if this is a cheap operation, I can<br>
>>>> >> > create<br>
>>>> >> > one<br>
>>>> >> > in the callback.<br>
>>>> >> ><br>
>>>> >> > Does this make the use clearer?<br>
>>>> >><br>
>>>> >> Yep, thx.<br>
>>>> >><br>
>>>> >> Cheers,<br>
>>>> >> /Manuel<br>
>>>> >><br>
>>>> >> > Thanks!<br>
>>>> >> ><br>
>>>> >> ><br>
>>>> >> > On Wed, Jul 11, 2012 at 12:29 AM, Manuel Klimek <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>>>> >> > wrote:<br>
>>>> >> >><br>
>>>> >> >> On Wed, Jul 11, 2012 at 6:03 AM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>><br>
>>>> >> >> wrote:<br>
>>>> >> >> > I'm trying to port my code to take advantage of matchers, now<br>
>>>> >> >> > that<br>
>>>> >> >> > they<br>
>>>> >> >> > are<br>
>>>> >> >> > in mainline. Some of the work I want to do involves semantic<br>
>>>> >> >> > analysis<br>
>>>> >> >> > of<br>
>>>> >> >> > the<br>
>>>> >> >> > results (i.e. in the callback). What would be the best way to<br>
>>>> >> >> > get a<br>
>>>> >> >> > Sema<br>
>>>> >> >> > or<br>
>>>> >> >> > CompilerInstance out of either RefactoringTool or MatchResult?<br>
>>>> >> >> > I'm<br>
>>>> >> >> > currently<br>
>>>> >> >> > playing with changing MatchASTConsumer to inherit from<br>
>>>> >> >> > SemaConsumer,<br>
>>>> >> >> > so<br>
>>>> >> >> > that<br>
>>>> >> >> > MatchFinder can track a Sema object the same way it does an<br>
>>>> >> >> > ASTContext.<br>
>>>> >> >><br>
>>>> >> >> I'd be interested in what the use cases for the semantic analysis<br>
>>>> >> >> are<br>
>>>> >> >> first. Often it seems like it would be better to have the<br>
>>>> >> >> information<br>
>>>> >> >> available in the AST instead of rerunning (potentially expensive)<br>
>>>> >> >> semantic analysis.<br>
>>>> >> >><br>
>>>> >> >> Cheers,<br>
>>>> >> >> /Manuel<br>
>>>> >> >><br>
>>>> >> >> ><br>
>>>> >> >> > Thanks!<br>
>>>> >> >> ><br>
>>>> >> >> ><br>
>>>> >> >> > On Mon, Jul 2, 2012 at 7:22 AM, Manuel Klimek<br>
>>>> >> >> > <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>>>> >> >> > wrote:<br>
>>>> >> >> >><br>
>>>> >> >> >> On Mon, Jul 2, 2012 at 4:16 PM, Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>><br>
>>>> >> >> >> wrote:<br>
>>>> >> >> >>><br>
>>>> >> >> >>> On Sun, Jul 1, 2012 at 10:45 PM, Manuel Klimek<br>
>>>> >> >> >>> <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>>>> >> >> >>> wrote:<br>
>>>> >> >> >>>><br>
>>>> >> >> >>>> On Fri, Jun 29, 2012 at 8:17 PM, Sam Panzer<br>
>>>> >> >> >>>> <<a href="mailto:panzer@google.com">panzer@google.com</a>><br>
>>>> >> >> >>>> wrote:<br>
>>>> >> >> >>>>><br>
>>>> >> >> >>>>> Thanks for the input!<br>
>>>> >> >> >>>>><br>
>>>> >> >> >>>>> Tooling/Refactoring is definitely the right way to go -<br>
>>>> >> >> >>>>> dumping<br>
>>>> >> >> >>>>> to<br>
>>>> >> >> >>>>> stdout was just a holdover from the example on LibTooling.<br>
>>>> >> >> >>>>> I'll<br>
>>>> >> >> >>>>> change it<br>
>>>> >> >> >>>>> once I figure out how it works - and a clean way to arrange<br>
>>>> >> >> >>>>> the<br>
>>>> >> >> >>>>> tests.<br>
>>>> >> >> >>>>><br>
>>>> >> >> >>>>> As for the use of matchers vs. visitors, I decided to use a<br>
>>>> >> >> >>>>> visitor<br>
>>>> >> >> >>>>> because this is a relatively complex transformation. I would<br>
>>>> >> >> >>>>> happily<br>
>>>> >> >> >>>>> use<br>
>>>> >> >> >>>>> matchers if I thought I could - and I think that some other<br>
>>>> >> >> >>>>> c++11<br>
>>>> >> >> >>>>> migrations<br>
>>>> >> >> >>>>> can easily be written with matchers - but I think the for<br>
>>>> >> >> >>>>> loop<br>
>>>> >> >> >>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>> I'm not claiming that the matchers needed to match those<br>
>>>> >> >> >>>> constructs<br>
>>>> >> >> >>>> are<br>
>>>> >> >> >>>> all already written - but if we write the questions you need<br>
>>>> >> >> >>>> to<br>
>>>> >> >> >>>> ask<br>
>>>> >> >> >>>> into<br>
>>>> >> >> >>>> matchers, other people who want to match similar things can<br>
>>>> >> >> >>>> reuse<br>
>>>> >> >> >>>> them, thus<br>
>>>> >> >> >>>> amplifying the impact of the code you write ;)<br>
>>>> >> >> >>>><br>
>>>> >> >> >>>>><br>
>>>> >> >> >>>>> checks need some features that matchers don't have (correct<br>
>>>> >> >> >>>>> me if<br>
>>>> >> >> >>>>> I'm<br>
>>>> >> >> >>>>> wrong!). For example, the check for statically allocated<br>
>>>> >> >> >>>>> array-based<br>
>>>> >> >> >>>>> loops<br>
>>>> >> >> >>>>> does this:<br>
>>>> >> >> >>>>> Given a for loop, determine if:<br>
>>>> >> >> >>>>>  - The increment statement increments (via ++) exactly one<br>
>>>> >> >> >>>>> integral<br>
>>>> >> >> >>>>> index variable, such that the variable is declared and<br>
>>>> >> >> >>>>> initialized<br>
>>>> >> >> >>>>> to zero<br>
>>>> >> >> >>>>> in the init portion of the loop, and that this variable's<br>
>>>> >> >> >>>>> value<br>
>>>> >> >> >>>>> is a<br>
>>>> >> >> >>>>> compared (via <, > or !=) to a nonnegative compile-time<br>
>>>> >> >> >>>>> constant<br>
>>>> >> >> >>>>> N<br>
>>>> >> >> >>>>> in the<br>
>>>> >> >> >>>>> compare portion.<br>
>>>> >> >> >>>>>  - The index variable is only ever used in an<br>
>>>> >> >> >>>>> ArrayIndexExpession<br>
>>>> >> >> >>>>> indexing a single, statically allocated array A.<br>
>>>> >> >> >>>>>  - The array A has exactly N elements.<br>
>>>> >> >> >>>>>  - Additionally, if the ArrayIndexExpression A[index] is<br>
>>>> >> >> >>>>> ever<br>
>>>> >> >> >>>>> assigned,<br>
>>>> >> >> >>>>> passed to a function or copied as a non-const reference, or<br>
>>>> >> >> >>>>> its<br>
>>>> >> >> >>>>> address<br>
>>>> >> >> >>>>> taken with & (I still need to add a check for calls to<br>
>>>> >> >> >>>>> non-const<br>
>>>> >> >> >>>>> member<br>
>>>> >> >> >>>>> functions), the loop variable in the converted version needs<br>
>>>> >> >> >>>>> to<br>
>>>> >> >> >>>>> be a<br>
>>>> >> >> >>>>> non-const reference so that the value will be correctly<br>
>>>> >> >> >>>>> updated<br>
>>>> >> >> >>>>> (this step<br>
>>>> >> >> >>>>> adds the most complexity).<br>
>>>> >> >> >>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>> ... and the matcher I would want to write for this looks<br>
>>>> >> >> >>>> something<br>
>>>> >> >> >>>> like<br>
>>>> >> >> >>>> that:<br>
>>>> >> >> >>>> ForLoop(<br>
>>>> >> >> >>>>   HasInitialization(Declaration(Id("loopvar",<br>
>>>> >> >> >>>> HasType(IsIntegral())))),<br>
>>>> >> >> >>>>   HasCondition(BinaryOperator(<br>
>>>> >> >> >>>>     HasAnyOperand(DeclarationReference(Id("condref",<br>
>>>> >> >> >>>> To(Variable())))),<br>
>>>> >> >> >>>>     HasAnyOperand(IntegerLiteral()))),<br>
>>>> >> >> >>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>> HasIncrement(UnaryOperator(HasUnaryOperand(DeclarationReference(Id("incref",<br>
>>>> >> >> >>>> To(Variable()))))), ...),<br>
>>>> >> >> >>>> )<br>
>>>> >> >> >>>><br>
>>>> >> >> >>>> In general, the complex stuff can stay complex, but the<br>
>>>> >> >> >>>> simple<br>
>>>> >> >> >>>> stuff<br>
>>>> >> >> >>>> shouldn't be lots of code.<br>
>>>> >> >> >>><br>
>>>> >> >> >>><br>
>>>> >> >> >>> Good point - and this is much easier to read than the<br>
>>>> >> >> >>> equivalent<br>
>>>> >> >> >>> code<br>
>>>> >> >> >>> I<br>
>>>> >> >> >>> had written.<br>
>>>> >> >> >>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>>><br>
>>>> >> >> >>>>> The other types of loop (iterator-based, and array-like<br>
>>>> >> >> >>>>> container)<br>
>>>> >> >> >>>>> are<br>
>>>> >> >> >>>>> more complicated to detect, since there are more permitted<br>
>>>> >> >> >>>>> ways<br>
>>>> >> >> >>>>> to<br>
>>>> >> >> >>>>> define<br>
>>>> >> >> >>>>> and use the index/iterators. What makes this difficult to do<br>
>>>> >> >> >>>>> entirely with<br>
>>>> >> >> >>>>> matchers is the number of back- and cross-references, as<br>
>>>> >> >> >>>>> well as<br>
>>>> >> >> >>>>> the<br>
>>>> >> >> >>>>> different local behaviors based on semantic properties. On<br>
>>>> >> >> >>>>> the<br>
>>>> >> >> >>>>> other<br>
>>>> >> >> >>>>> hand,<br>
>>>> >> >> >>>>> if there were some kind of backreference-enabled matcher<br>
>>>> >> >> >>>>> that<br>
>>>> >> >> >>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>> The way to handle the back-refs is to bind the nodes you<br>
>>>> >> >> >>>> want, and<br>
>>>> >> >> >>>> then<br>
>>>> >> >> >>>> pull them out and compare them in the callback.<br>
>>>> >> >> >>><br>
>>>> >> >> >>><br>
>>>> >> >> >>> I see - make the matcher slightly more general, then filter<br>
>>>> >> >> >>> the<br>
>>>> >> >> >>> results,<br>
>>>> >> >> >>> perhaps with a visitor.<br>
>>>> >> >> >>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>> Thoughts?<br>
>>>> >> >> >>>> /Manuel<br>
>>>> >> >> >>><br>
>>>> >> >> >>><br>
>>>> >> >> >>> This sounds like it would make at least half the work much<br>
>>>> >> >> >>> easier,<br>
>>>> >> >> >>> so<br>
>>>> >> >> >>> I<br>
>>>> >> >> >>> think it would definitely be worth it to try switching to a<br>
>>>> >> >> >>> matcher-based<br>
>>>> >> >> >>> solution. When are matchers supposed to hit mainline (or some<br>
>>>> >> >> >>> extra<br>
>>>> >> >> >>> cloneable repo) :) ?<br>
>>>> >> >> >><br>
>>>> >> >> >><br>
>>>> >> >> >> Matchers are currently in<br>
>>>> >> >> >> ^cfe/branches/tooling/include/clang/ASTMatchers/...<br>
>>>> >> >> >><br>
>>>> >> >> >> I'm currently working on renaming them to camelCase from<br>
>>>> >> >> >> CamelCase;<br>
>>>> >> >> >> there's a Tool to help with the conversion though, so no<br>
>>>> >> >> >> problem in<br>
>>>> >> >> >> starting<br>
>>>> >> >> >> now ...<br>
>>>> >> >> >><br>
>>>> >> >> >> Cheers,<br>
>>>> >> >> >> /Manuel<br>
>>>> >> >> >><br>
>>>> >> >> >>><br>
>>>> >> >> >>><br>
>>>> >> >> >>> -Sam<br>
>>>> >> >> >>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>>>><br>
>>>> >> >> >>>>> allowed me to locate all matches in a given Stmt, it could<br>
>>>> >> >> >>>>> be<br>
>>>> >> >> >>>>> *much*<br>
>>>> >> >> >>>>> easier to express some parts of the logic, such as the first<br>
>>>> >> >> >>>>> step<br>
>>>> >> >> >>>>> in<br>
>>>> >> >> >>>>> the<br>
>>>> >> >> >>>>> above list. I also suspect that a single-Stmt matcher would<br>
>>>> >> >> >>>>> better<br>
>>>> >> >> >>>>> way to<br>
>>>> >> >> >>>>> handle the last step; currently I track  whether the visitor<br>
>>>> >> >> >>>>> is<br>
>>>> >> >> >>>>> looking at a<br>
>>>> >> >> >>>>> statement or expression which fits any of the<br>
>>>> >> >> >>>>> const-disqualifying<br>
>>>> >> >> >>>>> conditions, and a note is made if I run into A[index].<br>
>>>> >> >> >>>>><br>
>>>> >> >> >>>>> Does this make the use case clearer? I don't really see a<br>
>>>> >> >> >>>>> better<br>
>>>> >> >> >>>>> way<br>
>>>> >> >> >>>>> to<br>
>>>> >> >> >>>>> approach this problem, but you guys know the available<br>
>>>> >> >> >>>>> toolkit<br>
>>>> >> >> >>>>> far<br>
>>>> >> >> >>>>> better<br>
>>>> >> >> >>>>> than I do.<br>
>>>> >> >> >>>>><br>
>>>> >> >> >>>>> On Fri, Jun 29, 2012 at 2:48 AM, Manuel Klimek<br>
>>>> >> >> >>>>> <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>>>> >> >> >>>>> wrote:<br>
>>>> >> >> >>>>>><br>
>>>> >> >> >>>>>> On Fri, Jun 29, 2012 at 11:45 AM, Chandler Carruth<br>
>>>> >> >> >>>>>> <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:<br>
>>>> >> >> >>>>>>><br>
>>>> >> >> >>>>>>> I tend to agree that this should use the<br>
>>>> >> >> >>>>>>> Tooling/Refactoring<br>
>>>> >> >> >>>>>>> stuff.<br>
>>>> >> >> >>>>>>><br>
>>>> >> >> >>>>>>> However, I'm curious about the best way to structure the<br>
>>>> >> >> >>>>>>> location<br>
>>>> >> >> >>>>>>> of<br>
>>>> >> >> >>>>>>> migration candidates: AST matchers vs. visitor.<br>
>>>> >> >> >>>>>>><br>
>>>> >> >> >>>>>>> I can almost see the visitor pattern working really well<br>
>>>> >> >> >>>>>>> here<br>
>>>> >> >> >>>>>>> as<br>
>>>> >> >> >>>>>>> each<br>
>>>> >> >> >>>>>>> different construct can have a pile of migration logic<br>
>>>> >> >> >>>>>>> dropped<br>
>>>> >> >> >>>>>>> in.... But if<br>
>>>> >> >> >>>>>>> there is a need to connect dots between more distant<br>
>>>> >> >> >>>>>>> constructs,<br>
>>>> >> >> >>>>>>> that<br>
>>>> >> >> >>>>>>> wouldn't work so well.... Not at all sure what would be<br>
>>>> >> >> >>>>>>> best<br>
>>>> >> >> >>>>>>> here.<br>
>>>> >> >> >>>>>><br>
>>>> >> >> >>>>>><br>
>>>> >> >> >>>>>> I've used a combination before - use matchers for the stuff<br>
>>>> >> >> >>>>>> where<br>
>>>> >> >> >>>>>> they<br>
>>>> >> >> >>>>>> work well, then write a very small easy-to-understand<br>
>>>> >> >> >>>>>> visitor if<br>
>>>> >> >> >>>>>> you need<br>
>>>> >> >> >>>>>> more. I think that brings down code size by quite a bit -<br>
>>>> >> >> >>>>>> obviously<br>
>>>> >> >> >>>>>> just a<br>
>>>> >> >> >>>>>> gut feeling here.<br>
>>>> >> >> >>>>>><br>
>>>> >> >> >>>>>>><br>
>>>> >> >> >>>>>>><br>
>>>> >> >> >>>>>>><br>
>>>> >> >> >>>>>>> On Fri, Jun 29, 2012 at 1:37 AM, Manuel Klimek<br>
>>>> >> >> >>>>>>> <<a href="mailto:klimek@google.com">klimek@google.com</a>><br>
>>>> >> >> >>>>>>> wrote:<br>
>>>> >> >> >>>>>>>><br>
>>>> >> >> >>>>>>>> On Fri, Jun 29, 2012 at 4:06 AM, Sam Panzer<br>
>>>> >> >> >>>>>>>> <<a href="mailto:panzer@google.com">panzer@google.com</a>><br>
>>>> >> >> >>>>>>>> wrote:<br>
>>>> >> >> >>>>>>>>><br>
>>>> >> >> >>>>>>>>> In case anyone wanted to take a look, the attached patch<br>
>>>> >> >> >>>>>>>>> includes<br>
>>>> >> >> >>>>>>>>> the tool I've been working on. I create a new binary,<br>
>>>> >> >> >>>>>>>>> c++migrate, which<br>
>>>> >> >> >>>>>>>>> attempts to convert for loops in the source files given<br>
>>>> >> >> >>>>>>>>> to<br>
>>>> >> >> >>>>>>>>> it.<br>
>>>> >> >> >>>>>>>>> Most of my<br>
>>>> >> >> >>>>>>>>> focus has been on the FrontedAction, so I skirted all of<br>
>>>> >> >> >>>>>>>>> the<br>
>>>> >> >> >>>>>>>>> issues<br>
>>>> >> >> >>>>>>>>> mentioned above by keeping the frontend interaction<br>
>>>> >> >> >>>>>>>>> minimal<br>
>>>> >> >> >>>>>>>>> (i.e. I just<br>
>>>> >> >> >>>>>>>>> call Tooling::ClangTool::run), and the changes are just<br>
>>>> >> >> >>>>>>>>> reported<br>
>>>> >> >> >>>>>>>>> on standard<br>
>>>> >> >> >>>>>>>>> output, if there are any to be made.<br>
>>>> >> >> >>>>>>>>><br>
>>>> >> >> >>>>>>>>> The tool can currently convert for loops that range over<br>
>>>> >> >> >>>>>>>>> (1)<br>
>>>> >> >> >>>>>>>>> statically allocated arrays, and (2) Clang-style<br>
>>>> >> >> >>>>>>>>> iterator-based<br>
>>>> >> >> >>>>>>>>> loops (with<br>
>>>> >> >> >>>>>>>>> begin and end iterators defined). All loop variables<br>
>>>> >> >> >>>>>>>>> need to<br>
>>>> >> >> >>>>>>>>> be<br>
>>>> >> >> >>>>>>>>> declared<br>
>>>> >> >> >>>>>>>>> within the loop's initialization step in order for it to<br>
>>>> >> >> >>>>>>>>> be<br>
>>>> >> >> >>>>>>>>> converted,<br>
>>>> >> >> >>>>>>>>> though this requirement can potentially be eliminated.<br>
>>>> >> >> >>>>>>>>> I'm<br>
>>>> >> >> >>>>>>>>> working on<br>
>>>> >> >> >>>>>>>>> converting iterator-based loops that call<br>
>>>> >> >> >>>>>>>>> someContainer.end()<br>
>>>> >> >> >>>>>>>>> on<br>
>>>> >> >> >>>>>>>>> each<br>
>>>> >> >> >>>>>>>>> iteration, since they're probably the common case in<br>
>>>> >> >> >>>>>>>>> many<br>
>>>> >> >> >>>>>>>>> codebases.<br>
>>>> >> >> >>>>>>>>><br>
>>>> >> >> >>>>>>>>> Just for fun, I ran the tool over the 41 .cpp files in<br>
>>>> >> >> >>>>>>>>> lib/Sema,<br>
>>>> >> >> >>>>>>>>> and my tool found 71 convertible loops in 17 files.<br>
>>>> >> >> >>>>>>>>> There is<br>
>>>> >> >> >>>>>>>>> plenty more<br>
>>>> >> >> >>>>>>>>> work to go, because it clearly missed some easy ones.<br>
>>>> >> >> >>>>>>>>><br>
>>>> >> >> >>>>>>>>> Any input or feedback is welcome!<br>
>>>> >> >> >>>>>>>><br>
>>>> >> >> >>>>>>>><br>
>>>> >> >> >>>>>>>> High-level observations:<br>
>>>> >> >> >>>>>>>> 1. the handling of the rewrites; any reason not to use<br>
>>>> >> >> >>>>>>>> the<br>
>>>> >> >> >>>>>>>> Tooling/Refactoring stuff? Currently in the patch it<br>
>>>> >> >> >>>>>>>> looks to<br>
>>>> >> >> >>>>>>>> me<br>
>>>> >> >> >>>>>>>> like the<br>
>>>> >> >> >>>>>>>> files are not rewritten, but dumped to stdout<br>
>>>> >> >> >>>>>>>> 2. is the reason not to use the matchers here that<br>
>>>> >> >> >>>>>>>> they're not<br>
>>>> >> >> >>>>>>>> landed in mainline yet?<br>
>>>> >> >> >>>>>>>><br>
>>>> >> >> >>>>>>>> Cheers,<br>
>>>> >> >> >>>>>>>> /Manuel<br>
>>>> >> >> >>>>>>>><br>
>>>> >> >> >>>>>>>>><br>
>>>> >> >> >>>>>>>>><br>
>>>> >> >> >>>>>>>>> -Sam<br>
>>>> >> >> >>>>>>>>><br>
>>>> >> >> >>>>>>>>> On Thu, Jun 28, 2012 at 10:50 AM, Sam Panzer<br>
>>>> >> >> >>>>>>>>> <<a href="mailto:panzer@google.com">panzer@google.com</a>><br>
>>>> >> >> >>>>>>>>> wrote:<br>
>>>> >> >> >>>>>>>>>><br>
>>>> >> >> >>>>>>>>>> I'm that intern :)<br>
>>>> >> >> >>>>>>>>>><br>
>>>> >> >> >>>>>>>>>> -Sam<br>
>>>> >> >> >>>>>>>>>><br>
>>>> >> >> >>>>>>>>>><br>
>>>> >> >> >>>>>>>>>> On Wed, Jun 27, 2012 at 9:48 PM, John Wiegley<br>
>>>> >> >> >>>>>>>>>> <<a href="mailto:johnw@boostpro.com">johnw@boostpro.com</a>><br>
>>>> >> >> >>>>>>>>>> wrote:<br>
>>>> >> >> >>>>>>>>>>><br>
>>>> >> >> >>>>>>>>>>> >>>>> Sam Panzer <<a href="mailto:panzer@google.com">panzer@google.com</a>> writes:<br>
>>>> >> >> >>>>>>>>>>><br>
>>>> >> >> >>>>>>>>>>> > In particular, I am working on a tool to convert<br>
>>>> >> >> >>>>>>>>>>> > existing<br>
>>>> >> >> >>>>>>>>>>> > C++<br>
>>>> >> >> >>>>>>>>>>> > for loops to<br>
>>>> >> >> >>>>>>>>>>> > take advantage of the new C++11 range-based syntax.<br>
>>>> >> >> >>>>>>>>>>> > I can<br>
>>>> >> >> >>>>>>>>>>> > imagine similar<br>
>>>> >> >> >>>>>>>>>>> > tools to replace const with constexpr, macro hacks<br>
>>>> >> >> >>>>>>>>>>> > with<br>
>>>> >> >> >>>>>>>>>>> > static_assert, and<br>
>>>> >> >> >>>>>>>>>>> > potentially other common refactorings.<br>
>>>> >> >> >>>>>>>>>>><br>
>>>> >> >> >>>>>>>>>>> > Thoughts? Suggestions?<br>
>>>> >> >> >>>>>>>>>>><br>
>>>> >> >> >>>>>>>>>>> You really must watch this presentation, if you<br>
>>>> >> >> >>>>>>>>>>> haven't<br>
>>>> >> >> >>>>>>>>>>> already:<br>
>>>> >> >> >>>>>>>>>>><br>
>>>> >> >> >>>>>>>>>>>     <a href="http://www.youtube.com/watch?v=yuIOGfcOH0k" target="_blank">http://www.youtube.com/watch?v=yuIOGfcOH0k</a><br>
>>>> >> >> >>>>>>>>>>><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">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>
>>>> >> >> >>>>>>>>>><br>
>>>> >> >> >>>>>>>>><br>
>>>> >> >> >>>>>>>>><br>
>>>> >> >> >>>>>>>>> _______________________________________________<br>
>>>> >> >> >>>>>>>>> cfe-dev mailing list<br>
>>>> >> >> >>>>>>>>> <a href="mailto:cfe-dev@cs.uiuc.edu">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>
>>>> >> >> >>>>>>>><br>
>>>> >> >> >>>>>>>><br>
>>>> >> >> >>>>>>>> _______________________________________________<br>
>>>> >> >> >>>>>>>> cfe-dev mailing list<br>
>>>> >> >> >>>>>>>> <a href="mailto:cfe-dev@cs.uiuc.edu">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>
>>>> >> >> >>>>>>><br>
>>>> >> >> >>>>>><br>
>>>> >> >> >>>>><br>
>>>> >> >> >>>><br>
>>>> >> >> >>><br>
>>>> >> >> >><br>
>>>> >> >> ><br>
>>>> >> >> ><br>
>>>> >> >> > _______________________________________________<br>
>>>> >> >> > cfe-dev mailing list<br>
>>>> >> >> > <a href="mailto:cfe-dev@cs.uiuc.edu">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>
>>>> >> ><br>
>>>> >> ><br>
>>>> ><br>
>>>> ><br>
>>><br>
>>><br>
>><br>
><br>
</div></div></blockquote></div><br></div>