On 09/04/2013 11:05 PM, Eli Friedman wrote:
On Wed, Sep 4, 2013 at 4:24 AM, Enea Zaffanella <[email protected]
<mailto:[email protected]>> wrote:
When dumping the following C++11 chunk of code
$ cat chunk.cc
struct A { A(int, int); };
A a_paren( A(0,0) );
A a_range( A{0,0} );
the CXXTemporaryObjectExpr generated by A{0,0}
is missing a proper end location
(the source location is correctly set for A(0,0)).
$ clang -cc1 -std=c++11 -ast-dump chunk.cc
[...]
`-VarDecl 0x5a963e0 <line:3:1, col:19> a_range 'struct A'
`-CXXConstructExpr 0x5a96570 <col:3, col:19> 'struct A' 'void
(struct A &&) noexcept' elidable
`-MaterializeTemporaryExpr 0x5a96550 <col:12, <invalid sloc>>
'struct A' xvalue
`-CXXTemporaryObjectExpr 0x5a964d8 <col:12, <invalid sloc>>
'struct A' 'void (int, int)'
|-IntegerLiteral 0x5a96448 <col:14> 'int' 0
`-IntegerLiteral 0x5a96468 <col:16> 'int' 0
Please find attached a patch (with testcase) fixing this issue.
OK to commit?
+ if (Kind.getKind() == InitializationKind::IK_DirectList)
+ ParenRange = SourceRange(LBraceLoc, RBraceLoc);
+ else
ParenRange = Kind.getParenRange();
This isn't right: there aren't any parentheses. getParenRange() on a
CXXTemporaryObjectExpr shouldn't return source locations pointing at
tokens which aren't parentheses.
You should be able to fix this purely in
CXXTemporaryObjectExpr::getLocStart()/getLocEnd().
-Eli
I see your point but ...
My guess is that getParenRange() method was designed considering only
C++03; now that we also have C++11 list initialization, I was freely
reinterpreting it as meaning "getParenOrBraceRange()".
Note that, in the dump above, the arguments of the
CXXTemporaryObjectExpr node are the elements of the InitListExpr.
That is, the InitListExpr node is unwrapped before creating the
temporary object node. So, if the getParenRange() is interpreted to
strictly mean only parentheses (not braces), we end up having no place
to store the source location for the braces themselves (and hence no
place to store the end location of the node). That is, we can not query
the arguments to obtain the end location (which is the trick used in
CXXConstructExpr::getLocEnd).
To summarize, I see two options:
(a) rename getParenRange() as getParenOrBraceRange();
(b) have two different methods, getParenRange() and getBraceRange(), to
be used based on the value of a boolean flag.
Question: is it the case that
bool CXXConstructExpr::isListInitialization()
is such an appropriate flag to be used here?
I would probably opt for (b).
Do you see a third option which is better?
Enea.
P.S.: while playing on variations of the testcase in the patch, I
noticed the following:
# cat bug.cc
struct A {
A(const int(&)[1]);
};
A a_paren( A( {0} ) );
A a_range( A{ {0} } );
When dumping `a_paren', we see that A( {0} ) produces a
CXXFunctionaCastExpr node (which is fine).
In contrast, when dumping `a_range' we see that A{ {0} } produces a
CXXTemporaryObjectExpr node. This does not match the doxygen
documentation for this AST node, where it is said:
> Represents a C++ functional cast expression that builds a
> temporary object.
>
> This expression type represents a C++ "functional" cast
> (C++[expr.type.conv]) with N != 1 arguments that invokes a
> constructor to build a temporary object. With N == 1 arguments the
> functional cast expression will be represented by
> CXXFunctionalCastExpr.
Is the doxygen documentation obsolete? Or is it the case that a
CXXFunctionalCastExpr node should have been produced for `a_range' too?
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
