[cfe-commits] r155848 - in /cfe/trunk: include/clang/AST/ExprCXX.h lib/AST/ExprCXX.cpp lib/Serialization/ASTReaderStmt.cpp lib/Serialization/ASTWriterStmt.cpp

[Argyrios Kyrtzidis]

On May 1, 2012, at 2:24 PM, John McCall wrote:

> On May 1, 2012, at 1:21 PM, Argyrios Kyrtzidis wrote:
>> On Apr 30, 2012, at 6:57 PM, John McCall wrote:
>>> On Apr 30, 2012, at 3:12 PM, Argyrios Kyrtzidis wrote:
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=155848&view=rev
>>>> Log:
>>>> Store the source range of a CXXOperatorCallExpr in the Expr object instead of
>>>> calculating it recursively.
>>>> 
>>>> boost::assign::tuple_list_of uses the trick of chaining call operator expressions in order to declare a "list of tuples", e.g:
>>>> std::vector<tuple> v = boost::assign::tuple_list_of(1, "foo")(2, "bar")(3, "qqq");
>>>> 
>>>> Due to CXXOperatorCallExpr calculating its source range recursively we would get
>>>> significant slowdowns with a large number of chained call operator expressions and the
>>>> potential for stack overflow.
>>> 
>>> This is unfortunate.  It also shouldn't really be necessary:  for the call operator, computing the beginning source location should always be tail-recursive.  The only reason we can't tail-recurse here is that getSourceRange() has to compute the end location as well, but that value is actually being ignored by the caller in all the recursive cases.
>>> 
>>> Would you mind trying the following experiment?  Try making getSourceRange() call getLocStart() or getLocEnd() instead of getSourceRange().getBegin() or .getEnd(), and make sure that all the expressions in the "loop" have specialized getLocStart() / getLocEnd() implementations.  That includes CXXOperatorCallExpr (for which you make getSourceRange() just assemble the result out of getLocStart() and getLocEnd()).
>> 
>> It cut down time (pre-r155848) by ~30% but it is still way slow on my test case.
>> 
>> I attached the diff I made with your suggestions.
> 
> Thanks.  Please do at least make the computation use getLocStart() and getLocEnd() instead of getSourceRange().getBegin() / getEnd(), here and anywhere else you see.

Certainly, thanks for the tip!

> 
> John.