Chris Lattner wrote:
+/// CXXFunctionalCastExpr - [C++ 5.2.3p1] Explicit type conversion
+/// (functional notation).
+///
Please add an example (e.g. "int(0.5)") to the comment. Also, CXXFunctionalCastExpr has no out-of-line virtual functions, which means the compiler will splat vtables for it into every translation unit that references it. Please give it an out-of-line virtual function. There are probably other ASTs that are similarly naughty. 
Done.
+class CXXFunctionalCastExpr : public CastExpr {
Deriving from CastExpr means that this has a location for the lparen, which is somewhat unneeded. We also have a bit of strangeness because ImplicitCast and Cast are unrelated. As a follow-on patch, what do you think of this sort of class hierarchy:
Expr
-> CastExpr
-> ExplicitCastExpr
-> ImplicitCastExpr
-> CXXFunctionalCastExpr
CastExpr would just have a subexpr but no location. Does that seem reasonable?
This looks great! I've made CastExpr an abstract class and replaced its use with an ExplicitCastExpr here:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20080818/007043.html
+/// CXXZeroInitValueExpr - [C++ 5.2.3p2]
+/// Expression "T()" which creates a value-initialized Rvalue of type T.
+///
+class CXXZeroInitValueExpr : public Expr {
+ SourceLocation TyBeginLoc;
+ SourceLocation LParenLoc;
+ SourceLocation RParenLoc;
It might be worthwhile to point out that this is not used for constructors, by saying someting like "of non-class type T" or something if appropriate. Please don't include the lparen loc, we should add it on demand if some client needs it.
Done.
/// [OBJC] objc-string-literal
+/// [C++] simple-type-specifier '(' expression-list[opt] ')' [C++ 5.2.3]
+/// [C++] typename-specifier '(' expression-list[opt] ')' [TODO]
/// [C++] 'const_cast' '<' type-name '>' '(' expression ')' [C++ 5.2p1]
The C++ Spec lists these as postfix exprs, not primary exprs. Does it affect precedence or matter in practice?
No, they work fine. Here are some comments from ParseCastExpression:
// This handles all of cast-expression, unary-expression, postfix-expression,
// and primary-expression. We handle them together like this for efficiency
// and to simplify handling of an expression starting with a '(' token
+ case tok::identifier: {
+ if (getLang().CPlusPlus &&
+ Actions.isTypeName(*Tok.getIdentifierInfo(), CurScope))
+ goto HandleType;
Please add a comment describing what this is doing with a simple example, e.g. "Handle C++ constructor-style cast [e.g. "T(4.5)"] if T is a typedef for double"
Done.
+ // Match the ')'.
+ if (Tok.isNot(tok::r_paren)) {
+ MatchRHSPunctuation(tok::r_paren, LParenLoc);
+ return ExprResult(true);
+ }
+
+ SourceLocation RParenLoc = ConsumeParen();
Does this work instead?:
+ // Match the ')'.
+ SourceLocation RParenLoc = MatchRHSPunctuation(tok::r_paren, LParenLoc);
The intention of MatchRHSPunctuation is that you just unconditionally call it to simplify the caller.
Done.
+/// ParseCXXSimpleTypeSpecifier - [C++ 7.1.5.2] Simple type specifiers.
Please add a comment saying that this should only be called when the current token is known to be part of a simple type specifier. While this is somewhat implicit in the rest of the actions, for some reason I had to check for this one when I saw the identifier handling code.
Done.
+ // GNU typeof support.
+ case tok::kw_typeof:
+ ParseTypeofSpecifier(DS);
+ DS.Finish(Diags, PP.getSourceManager(), getLang());
+ return;
+ }
This doesn't call DS.SetRangeEnd? Does ParseTypeofSpecifier do this?
Yes, ParseTypeofSpecifier does it. I've made a small commit, before posting the patch, here:
http://lists.cs.uiuc.edu/pipermail/cfe-commits/Week-of-Mon-20080811/007003.html
+ // FIXME: Support for class constructors.
+ assert(0 && "No support for class constructors yet.");
+ abort();
Please make this output an error (e.g. unimplemented), it is nice for clang to never crash on any input, even if due to a partially implemented one.
Done.
+ // C++ 5.2.3p1:
+ // If the expression list is a single expression, the type conversion
+ // expression is equivalent (in definedness, and if defined in meaning) to the
+ // corresponding cast expression.
It seems that this check should happen before the check for recorddecl?
My intention was to do something like this:
if class type
-> do semantic checks, resolve constructors etc.
-> if single expr
-> create some class-specialized expr
else if multi expr
-> create some other class-specialized expr
......
Instead of:
if single expr
-> if non-class type
-> create CXXFunctionalCastExpr
-> else if class type
-> semantic checks, resolve constructor
-> create some class-specialized expr
Not a big difference though. Will probably be more clear what is better when constructors are in place and it's determined what kind of Exprs are needed to represent them.
New patch attached!
-Argiris
func-style-cast-2.patch (22.6 KB)