OK thank for the review I corrected the issues Here is an updated patch.
On Fri, Dec 3, 2010 at 2:05 PM, Douglas Gregor <[email protected]> wrote: > > On Dec 2, 2010, at 4:43 PM, Francois Pichet wrote: > >> Updated patch >> Correct some typos in comments. >> >> On Thu, Dec 2, 2010 at 7:14 PM, Francois Pichet <[email protected]> wrote: >>> Hi, >>> >>> This patch implements the __is_base_of type traits intrinsic. >>> A new AST node BinaryTypeTraitExpr is added. >>> >>> For documentation I used: >>> >>> http://msdn.microsoft.com/en-us/library/ms177194(v=VS.90).aspx >>> http://gcc.gnu.org/onlinedocs/gcc/Type-Traits.html >>> C++ standard N3126 > > Index: include/clang/AST/ExprCXX.h > =================================================================== > --- include/clang/AST/ExprCXX.h (revision 120744) > +++ include/clang/AST/ExprCXX.h (working copy) > @@ -1452,6 +1452,70 @@ > friend class ASTStmtReader; > }; > > +/// BinaryTypeTraitExpr - A GCC or MS binary type trait, as used in the > +/// implementation of TR1/C++0x type trait templates. > +/// Example: > +/// __is_base_of(Base, Derived) == true > +class BinaryTypeTraitExpr : public Expr { > + /// BTT - The trait. A BinaryTypeTrait enum in MSVC compat unsigned. > + unsigned BTT : 31; > + /// The value of the type trait. Unspecified if dependent. > + bool Value : 1; > + > + /// Loc - The location of the type trait keyword. > + SourceLocation Loc; > + > + /// RParen - The location of the closing paren. > + SourceLocation RParen; > + > + /// The lhs type being queried. > + TypeSourceInfo *LhsType; > + > + /// The rhs type being queried. > + TypeSourceInfo *RhsType; > + > +public: > + BinaryTypeTraitExpr(SourceLocation loc, BinaryTypeTrait btt, > + TypeSourceInfo *lhsType, TypeSourceInfo *rhsType, > + bool value, SourceLocation rparen, QualType ty) > + : Expr(BinaryTypeTraitExprClass, ty, VK_RValue, OK_Ordinary, false, > + lhsType->getType()->isDependentType()), > > BinaryTypeTraitExpr is value-dependent when either lhsType or rhsType is > dependent. > > + bool getValue() const { return Value; } > + > > assert() that this expression is not value-dependent? > > Index: lib/AST/StmtPrinter.cpp > =================================================================== > --- lib/AST/StmtPrinter.cpp (revision 120744) > +++ lib/AST/StmtPrinter.cpp (working copy) > @@ -1225,11 +1225,24 @@ > } > } > > +static const char *getTypeTraitName(BinaryTypeTrait BTT) { > + switch (BTT) { > + default: assert(false && "Unknown type trait"); > + case BTT_IsBaseOf: return "__is_base_of"; > + } > +} > + > > Please eliminate the default case and change the assert() to an > llvm_unreachable("Unknown binary type trait"); followed by "return "";". > > (Reason: default cases suppress GCC and Clang's warnings about not handling > all of the enumerators when switching on an enumeration. We want that warning > to tell us if we missed a case when we add a new binary type trait). > > Index: lib/Parse/ParseExprCXX.cpp > =================================================================== > --- lib/Parse/ParseExprCXX.cpp (revision 120744) > +++ lib/Parse/ParseExprCXX.cpp (working copy) > @@ -1820,6 +1820,13 @@ > } > } > > +static BinaryTypeTrait BinaryTypeTraitFromTokKind(tok::TokenKind kind) { > + switch(kind) { > + default: assert(false && "Not a known binary type trait."); > + case tok::kw___is_base_of: return BTT_IsBaseOf; > + } > +} > + > /// ParseUnaryTypeTrait - Parse the built-in unary type-trait > /// pseudo-functions that allow implementation of the TR1/C++0x type traits > /// templates. > @@ -1848,6 +1855,40 @@ > return Actions.ActOnUnaryTypeTrait(UTT, Loc, Ty.get(), RParen); > } > > +/// ParseBinaryTypeTrait - Parse the built-in binary type-trait > +/// pseudo-functions that allow implementation of the TR1/C++0x type traits > +/// templates. > +/// > +/// primary-expression: > +/// [GNU] binary-type-trait '(' type-id ',' type-id ')' > +/// > +ExprResult Parser::ParseBinaryTypeTrait() { > + BinaryTypeTrait BTT = BinaryTypeTraitFromTokKind(Tok.getKind()); > + SourceLocation Loc = ConsumeToken(); > + > + SourceLocation LParen = Tok.getLocation(); > + if (ExpectAndConsume(tok::l_paren, diag::err_expected_lparen)) > + return ExprError(); > + > + // FIXME: Error reporting absolutely sucks! If the this fails to parse a > type > + // there will be cryptic errors about mismatched parentheses and missing > + // specifiers. > + TypeResult LhsTy = ParseTypeName(); > + if (LhsTy.isInvalid()) > + return ExprError(); > + > + if (ExpectAndConsume(tok::comma, diag::err_expected_comma)) > + return ExprError(); > + > + TypeResult RhsTy = ParseTypeName(); > + if (RhsTy.isInvalid()) > + return ExprError(); > + > + SourceLocation RParen = MatchRHSPunctuation(tok::r_paren, LParen); > + > + return Actions.ActOnBinaryTypeTrait(BTT, Loc, LhsTy.get(), RhsTy.get(), > RParen); > +} > + > > > From a recovery standpoint, I suggest SkipUntil(tok::r_paren) when we fail to > parse either of the type names or are missing the comma. That way, we'll make > sure our parentheses are balanced. > > Index: lib/Sema/SemaExprCXX.cpp > =================================================================== > --- lib/Sema/SemaExprCXX.cpp (revision 120744) > +++ lib/Sema/SemaExprCXX.cpp (working copy) > @@ -2333,6 +2333,76 @@ > RParen, Context.BoolTy)); > } > > +ExprResult Sema::ActOnBinaryTypeTrait(BinaryTypeTrait BTT, > + SourceLocation KWLoc, > + ParsedType LhsTy, > + ParsedType RhsTy, > + SourceLocation RParen) { > + TypeSourceInfo *LhsTSInfo; > + QualType LhsT = GetTypeFromParser(LhsTy, &LhsTSInfo); > + if (!LhsTSInfo) > + LhsTSInfo = Context.getTrivialTypeSourceInfo(LhsT); > + > + TypeSourceInfo *RhsTSInfo; > + QualType RhsT = GetTypeFromParser(RhsTy, &RhsTSInfo); > + if (!RhsTSInfo) > + RhsTSInfo = Context.getTrivialTypeSourceInfo(RhsT); > + > + return BuildBinaryTypeTrait(BTT, KWLoc, LhsTSInfo, RhsTSInfo, RParen); > +} > + > +static bool EvaluateBinaryTypeTrait(Sema &Self, BinaryTypeTrait BTT, > + QualType LhsT, QualType RhsT, > + SourceLocation KeyLoc) { > + assert((!LhsT->isDependentType() || RhsT->isDependentType()) && > + "Cannot evaluate traits for dependent types."); > + > + switch(BTT) { > + default: assert(false && "Unknown type trait or not implemented"); > + case BTT_IsBaseOf: > + // C++ 20.7.5 p2 > + // Base is a base class of Derived without regard to cv-qualifiers or > + // Base and Derived are not unions and name the same class type without > + // regard to cv-qualifiers. > + if (Self.IsDerivedFrom(RhsT, LhsT) || > + (!LhsT->isUnionType() && !RhsT->isUnionType() > + && LhsT->getAsCXXRecordDecl() == RhsT->getAsCXXRecordDecl())) > + return true; > + > + return false; > + } > +} > + > > Same comment as before about the "default" case. > > + > +ExprResult Sema::BuildBinaryTypeTrait(BinaryTypeTrait BTT, > + SourceLocation KWLoc, > + TypeSourceInfo *LhsTSInfo, > + TypeSourceInfo *RhsTSInfo, > + SourceLocation RParen) { > + QualType LhsT = LhsTSInfo->getType(); > + QualType RhsT = RhsTSInfo->getType(); > + > + if (BTT == BTT_IsBaseOf) { > + // C++ 20.7.5 p2 > + // If Base and Derived are class types and are different types > + // (ignoring possible cv-qualifiers) then Derived shall be a complete > + // type. [] > > It's a silly nit, but I really prefer references to section names, e.g., > > C++0x [meta.rel]p2: > > + CXXRecordDecl *LhsDecl = LhsT->getAsCXXRecordDecl(); > + CXXRecordDecl *RhsDecl = RhsT->getAsCXXRecordDecl(); > + if (LhsDecl && RhsDecl && LhsDecl != RhsDecl && > + RequireCompleteType(KWLoc, RhsT, > + > diag::err_incomplete_type_used_in_type_trait_expr)) > + return ExprError(); > + } > > This doesn't quite do what you want, because getAsCXXRecordDecl() can return > the "current instantiation" within a class template, which of course isn't a > complete type. (It's wrong to even ask the question "are you a complete > type?" on a dependent type). > > I suggest querying the described properties directly: if we're in C++ and the > types are non-dependent, different class types, then require RhsT to be > complete. > > Index: test/SemaCXX/type-traits.cpp > =================================================================== > --- test/SemaCXX/type-traits.cpp (revision 120744) > +++ test/SemaCXX/type-traits.cpp (working copy) > @@ -446,3 +446,34 @@ > int t22[F(__has_virtual_destructor(void))]; > int t23[F(__has_virtual_destructor(cvoid))]; > } > + > + > +class Base {}; > +class Derived : Base {}; > +class Derived2a : Derived {}; > +class Derived2b : Derived {}; > +class Derived3 : virtual Derived2a, virtual Derived2b {}; > +template<typename T> struct BaseA { T a; }; > +template<typename T> struct DerivedB : BaseA<T> { }; > +template<typename T> struct CrazyDerived : T { }; > + > + > +class class_foward; // expected-note {{forward declaration of > 'class_foward'}} > > "class_forward", perhaps? > > It would be nice to have some tests involving template instantiation. > > Thanks for working on this! > > - Doug
is_base_of2.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
