[cfe-commits] [PATCH][Review request] - __is_base_of compiler intrisics

Douglas Gregor dgregor at apple.com
Fri Dec 3 11:05:21 PST 2010


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



More information about the cfe-commits mailing list