[cfe-commits] [PATCH] Fix ParenListExpr with null type

[Douglas Gregor]

On Jun 14, 2011, at 6:06 PM, Manuel Klimek wrote:

> The attached patch
> - fixes the introduction of null types for ParenListExpr's that end up
> in the AST for explicit initializers by making the constructor of
> ParenListExpr take a type (as suggested by dgregor on irc)
> - gets rid of some code I assume is dead (tested by running the tests
> and by running it over all of our internal C++ code without hitting
> any of those asserts - and by my inability to come up with an example
> that hits that code path (which admittedly doesn't mean a lot))
> - tested that the other in-use cases that create ParenListExpr's
> without a type don't lead to them being in the AST in the end

Index: lib/Sema/SemaDeclCXX.cpp
diff --git a/lib/Sema/SemaDeclCXX.cpp b/lib/Sema/SemaDeclCXX.cpp
index ce99efbd0bd2020f702da71fc87a80d0b86759a8..a95ef69085526291eb7532aeb9788f68bab235f6 100644
--- a/lib/Sema/SemaDeclCXX.cpp
+++ b/lib/Sema/SemaDeclCXX.cpp
@@ -1617,7 +1617,7 @@ Sema::BuildMemberInitializer(ValueDecl *Member, Expr **Args,
     // Can't check initialization for a member of dependent type or when
     // any of the arguments are type-dependent expressions.
     Init = new (Context) ParenListExpr(Context, LParenLoc, Args, NumArgs,
-                                       RParenLoc);
+                                       RParenLoc, QualType());
 
     // Erase any temporaries within this evaluation context; we're not
     // going to track them in the AST, since we'll be rebuilding the

Why not just use Member->getType() as the type of the ParenListExpr, so it never has a NULL type?

@@ -1659,7 +1659,7 @@ Sema::BuildMemberInitializer(ValueDecl *Member, Expr **Args,
     // and this approach is far more likely to get the corner cases right.
     if (CurContext->isDependentContext())
       Init = new (Context) ParenListExpr(Context, LParenLoc, Args, NumArgs,
-                                               RParenLoc);
+                                         RParenLoc, QualType());

Same question here, and in a number of places below. It seems like, if we're going to fix this, we should always make sure that ParenListExpr has a type.

Index: lib/Sema/SemaInit.cpp
diff --git a/lib/Sema/SemaInit.cpp b/lib/Sema/SemaInit.cpp
index a33f5d0b2f3ef530c689a2ddc28ebaef3074635e..f0d39893a449e62902f21160f10bd6a400353789 100644
--- a/lib/Sema/SemaInit.cpp
+++ b/lib/Sema/SemaInit.cpp
@@ -3684,19 +3684,9 @@ InitializationSequence::Perform(Sema &S,
 
       }
     }
-
-    if (Kind.getKind() == InitializationKind::IK_Copy || Kind.isExplicitCast())
-      return ExprResult(Args.release()[0]);
-
-    if (Args.size() == 0)
-      return S.Owned((Expr *)0);
-
-    unsigned NumArgs = Args.size();
-    return S.Owned(new (S.Context) ParenListExpr(S.Context,
-                                                 SourceLocation(),
-                                                 (Expr **)Args.release(),
-                                                 NumArgs,
-                                                 SourceLocation()));
+    assert(Kind.getKind() == InitializationKind::IK_Copy ||
+           Kind.isExplicitCast());
+    return ExprResult(Args.release()[0]);
   }
 
   // No steps means no initialization.

Hrm, interesting. This is only dead because its potential callers seem to avoid building InitializationSequences in dependent cases. Please update the assert to always check that Args.size() ==1, and if *that* doesn't trigger any failures, this change is okay. We can always resurrect the code if the callers change.

	- Doug