[cfe-commits] [PATCH] C1X: generic selections, static asserts

Douglas Gregor dgregor at apple.com
Sat Mar 26 17:06:47 PDT 2011


On Mar 16, 2011, at 8:16 PM, Peter Collingbourne wrote:

> This patch series implements support for C1X generic selections
> and static assertions.
> 
> As an extension, generic selection support has been added for all
> supported languages.  The syntax is the same as for C1X, except that
> the keyword is "__generic".

Why not just enable it as _Generic? Both __generic and _Generic are reserved for the implementation anyway.

--- a/include/clang/AST/Expr.h
+++ b/include/clang/AST/Expr.h
@@ -3672,6 +3672,83 @@ public:
 };
 
 
+class GenericSelectionExpr : public Expr {

A comment block here would be nice, with an example of how this expression is written in source code.

+  enum { CONTROLLING, END_EXPR };
+  TypeSourceInfo **AssocTypes;
+  Stmt **SubExprs;
+  unsigned NumAssocs, ResultOffset;
+  SourceLocation GenericLoc, DefaultLoc, RParenLoc;

@@ -2639,6 +2682,48 @@ void ShuffleVectorExpr::setExprs(ASTContext &C, Expr ** Exprs,
   memcpy(SubExprs, Exprs, sizeof(Expr *) * NumExprs);
 }
 
+GenericSelectionExpr::GenericSelectionExpr(ASTContext &Context,
+                               SourceLocation GenericLoc, Expr *ControllingExpr,
+                               TypeSourceInfo **_AssocTypes, Expr **AssocExprs,
+                               unsigned NumAssocs, SourceLocation DefaultLoc,
+                               SourceLocation RParenLoc, unsigned ResultOffset)
+  : Expr(GenericSelectionExprClass,
+         AssocExprs[ResultOffset]->getType(),
+         AssocExprs[ResultOffset]->getValueKind(),
+         AssocExprs[ResultOffset]->getObjectKind(),
+         AssocExprs[ResultOffset]->isTypeDependent(),
+         AssocExprs[ResultOffset]->isValueDependent(),
+         AssocExprs[ResultOffset]->containsUnexpandedParameterPack()),
+    AssocTypes(new (Context) TypeSourceInfo*[NumAssocs]),
+    SubExprs(new (Context) Stmt*[END_EXPR+NumAssocs]), NumAssocs(NumAssocs),
+    ResultOffset(ResultOffset), GenericLoc(GenericLoc), DefaultLoc(DefaultLoc),
+    RParenLoc(RParenLoc) {
+  SubExprs[CONTROLLING] = ControllingExpr;
+  std::copy(_AssocTypes, _AssocTypes+NumAssocs, AssocTypes);
+  std::copy(AssocExprs, AssocExprs+NumAssocs, SubExprs+END_EXPR);
+}

We're not allowed to have parameters named _AssocTypes :)

+GenericSelectionExpr::GenericSelectionExpr(ASTContext &Context,
+                               SourceLocation GenericLoc, Expr *ControllingExpr,
+                               TypeSourceInfo **_AssocTypes, Expr **AssocExprs,
+                               unsigned NumAssocs, SourceLocation DefaultLoc,
+                               SourceLocation RParenLoc)
+  : Expr(GenericSelectionExprClass,
+         Context.DependentTy,
+         VK_RValue,
+         OK_Ordinary,
+         /*isTypeDependent=*/  true,
+         /*isValueDependent=*/ true,
+         /*containsUnexpandedParameterPack=*/ false),

This last bit is actually interesting... do we check for unexpanded parameter packs in Sema? It doesn't look like we do.

@@ -1797,6 +1801,82 @@ ExprResult Parser::ParseStringLiteralExpression() {
   return Actions.ActOnStringLiteral(&StringToks[0], StringToks.size());
 }
 
+/// ParseGenericSelectionExpression - Parse a C1X generic-selection.
+ExprResult Parser::ParseGenericSelectionExpression() {
+  assert(Tok.is(tok::kw__Generic) && "_Generic keyword expected");
+  SourceLocation KeyLoc = Tok.getLocation();
+  ConsumeToken();
+
+  SourceLocation LParenLoc = Tok.getLocation();
+  if (ExpectAndConsume(tok::l_paren, diag::err_expected_lparen, ""))
+    return ExprError();
+
+  ExprResult ControllingExpr;
+  {
+    EnterExpressionEvaluationContext Unevaluated(Actions, Sema::Unevaluated);
+    ControllingExpr = ParseAssignmentExpression();
+    if (ControllingExpr.isInvalid()) {
+      SkipUntil(tok::r_paren);
+      return ExprError();
+    }
+  }
+
+  if (ExpectAndConsume(tok::comma, diag::err_expected_comma, "")) {
+    SkipUntil(tok::r_paren);
+    return ExprError();
+  }
+
+  SourceLocation DefaultLoc;
+  TypeVector Types(Actions);
+  ExprVector Exprs(Actions);
+  while (1) {
+    ParsedType Ty;
+    if (Tok.is(tok::kw_default)) {
+      if (!DefaultLoc.isInvalid()) {
+        Diag(Tok, diag::err_duplicate_default_assoc);
+        Diag(DefaultLoc, diag::note_previous_default_assoc);
+        SkipUntil(tok::r_paren);
+        return ExprError();
+      }
+      DefaultLoc = ConsumeToken();
+      Ty = ParsedType();
+    } else {
+      ColonProtectionRAIIObject X(*this);
+      TypeResult TR = ParseTypeName();
+      if (TR.isInvalid()) {
+        SkipUntil(tok::r_paren);
+        return ExprError();
+      }
+      Ty = TR.release();
+    }
+    Types.push_back(Ty);
+
+    if (ExpectAndConsume(tok::colon, diag::err_expected_colon, "")) {
+      SkipUntil(tok::r_paren);
+      return ExprError();
+    }
+
+    ExprResult ER(ParseAssignmentExpression());

Should we be parsing these expressions in a potentially potentially evaluated context, and only consider the chosen expression to be in a potentially evaluated context?

--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -701,6 +701,162 @@ QualType Sema::UsualArithmeticConversions(Expr *&lhsExpr, Expr *&rhsExpr,
 //===----------------------------------------------------------------------===//
 
 
+ExprResult
+Sema::ActOnGenericSelectionExpr(SourceLocation KeyLoc,
+                                SourceLocation DefaultLoc,
+                                SourceLocation RParenLoc,
+                                Expr *ControllingExpr,
+                                MultiTypeArg types,
+                                MultiExprArg exprs) {
+  unsigned NumAssocs = types.size();
+  assert(NumAssocs == exprs.size());
+
+  ParsedType *ParsedTypes = types.release();
+  Expr **Exprs = exprs.release();
+
+  TypeSourceInfo **Types = new TypeSourceInfo*[NumAssocs];
+  for (unsigned i = 0; i < NumAssocs; ++i) {
+    if (ParsedTypes[i])
+      (void) GetTypeFromParser(ParsedTypes[i], &Types[i]);
+    else
+      Types[i] = 0;
+  }
+

If the type ends up being NULL because of an error, we should return an ExprResult(). I guess there could be one NULL ParsedType, for the default case?

+      // C1X 6.5.1.1p2 "No two generic associations in the same generic
+      // selection shall specify compatible types."
+      for (unsigned j = i+1; j < NumAssocs; ++j)
+        if (Types[j] && !Types[j]->getType()->isDependentType())
+          if (Context.typesAreCompatible(Types[i]->getType(),
+                                         Types[j]->getType())) {
+            Diag(Types[j]->getTypeLoc().getBeginLoc(),
+                 diag::err_assoc_compatible_types)
+              << Types[j]->getTypeLoc().getSourceRange()
+              << Types[j]->getType()
+              << Types[i]->getType();
+            Diag(Types[i]->getTypeLoc().getBeginLoc(),
+                 diag::note_compat_assoc)
+              << Types[i]->getTypeLoc().getSourceRange()
+              << Types[i]->getType();
+            TypeErrorFound = true;
+          }
+    }

In C++, this could just be SmallPtrSet on the canonical types. Then we could cook up a pointless microbenchmark that shows that compiling C++ is faster than compiling C! 

+  for (unsigned i = 0; i < NumAssocs; ++i) {
+    if (Types[i] && Types[i]->getType()->isDependentType())
+      return CreateDependentGenericSelectionExpr(*this, KeyLoc, DefaultLoc,
+                                                 RParenLoc, ControllingExpr,
+                                                 Types, Exprs, NumAssocs);
+  }

You could skip this loop by keeping an "are any of the types dependent?" flag, which gets updated as we're checking each of the types in the first loop.

+  unsigned ResultOffset =
+    CompatOffsets.size() ? CompatOffsets[0] : DefaultOffset;
+
+  if (ResultOffset == -1U) {
+    // We strip parens here because the controlling expression is typically
+    // parenthesized in macro definitions.
+    ControllingExpr = ControllingExpr->IgnoreParens();
+    Diag(ControllingExpr->getLocStart(), diag::err_generic_sel_no_match)
+      << ControllingExpr->getSourceRange() << ControllingExpr->getType();
+    return ExprError();
+  }

Why the sentinel value for ResultOffset? This could just be:

	if (CompatOffsets.empty()) {
		// error
	}

@@ -5441,6 +5457,45 @@ TreeTransform<Derived>::TransformCharacterLiteral(CharacterLiteral *E) {
 
 template<typename Derived>
 ExprResult
+TreeTransform<Derived>::TransformGenericSelectionExpr(GenericSelectionExpr *E) {
+  ExprResult ControllingExpr =
+    getDerived().TransformExpr(E->getControllingExpr());
+  if (ControllingExpr.isInvalid())
+    return ExprError();
+
+  Expr **AssocExprs = new Expr*[E->getNumAssocs()];
+  TypeSourceInfo **AssocTypes = new TypeSourceInfo*[E->getNumAssocs()];

Please use SmallVectors here, so you don't have to do the delete operations below. (Note that this code leaks when the associated expression fails to instantiate!)

+  for (unsigned i = 0; i != E->getNumAssocs(); ++i) {
+    TypeSourceInfo *TS = E->getAssocTypeSourceInfo(i);
+    if (TS) {
+      TypeSourceInfo *AssocType = getDerived().TransformType(TS);
+      if (!AssocType)
+        return ExprError();
+      AssocTypes[i] = AssocType;
+    } else {
+      AssocTypes[i] = 0;
+    }
+
+    ExprResult AssocExpr = getDerived().TransformExpr(E->getAssocExpr(i));
+    if (AssocExpr.isInvalid())
+      return ExprError();
+    AssocExprs[i] = AssocExpr.release();
+  }
+
+  ExprResult ER = getDerived().RebuildGenericSelectionExpr(E->getGenericLoc(),
+                                                           E->getDefaultLoc(),
+                                                           E->getRParenLoc(),
+                                                      ControllingExpr.release(),
+                                                           AssocTypes,
+                                                           AssocExprs,
+                                                           E->getNumAssocs());
+  delete AssocExprs;
+  delete AssocTypes;
+  return ER;
+}

The template metaprogrammer in me wonders if we should tweak the instantiation semantics in C++. In particular, we could go ahead and instantiate all the types first (since we have to). Then, only instantiate the expression corresponding to the chosen type, leaving the others uninstantiated. That would allow one to use _Generic as a guard, so that we only pick the expression that actually works.

Don't feel obligated to do this unless you also think it's cool :)

--- a/include/clang/Basic/TokenKinds.def
+++ b/include/clang/Basic/TokenKinds.def
@@ -236,6 +236,7 @@ KEYWORD(_Bool                       , KEYNOCXX)
 KEYWORD(_Complex                    , KEYALL)
 KEYWORD(_Generic                    , KEYC1X)
 KEYWORD(_Imaginary                  , KEYALL)
+KEYWORD(_Static_assert              , KEYC1X)
 KEYWORD(__func__                    , KEYALL)
 
 // C++ 2.11p1: Keywords.

Any particular reason why this isn't just an alias for "static_assert"?

Patches are looking great!

	- Doug



More information about the cfe-commits mailing list