[cfe-commits] [PATCH][Review request] - __is_base_of compiler intrisics
Francois Pichet
pichet2000 at gmail.com
Sat Dec 4 21:31:11 PST 2010
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 <dgregor at apple.com> 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 <pichet2000 at gmail.com> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: is_base_of2.patch
Type: application/octet-stream
Size: 26682 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20101205/38180d95/attachment.obj>
More information about the cfe-commits
mailing list