Sema: Let InitListExpr have dependent type instead of 'void'

[aaronpuchert]

Summary

It was always just a placeholder, but not a particularly good one: the
expression certainly has a value in most cases, and 'void' can also not
serve as an indicator for an undeduced type because of void{}.

Instead I think the dependent type is a good fit: an initializer list
is an expression that can instantiate to expressions of arbitrary types:
One could say that {...} is a map

T : Type → T, T ↦ T{...}

The type parameter can be explicitly specified or be deduced from an
implicit conversion target type. So one could for example view {1} as

struct InitList1 {
  template<typename T>
  operator T() { return T{1}; }
};

That in itself isn't type-dependent, but note that we let InitListExprs
have the actual type they initialize in the end, so in this case 'T'
instead of 'InitList1'.

A test failure in CXX/expr/expr.prim/expr.prim.lambda/p4.cpp revealed
that we were emitting errors "return type ... must match previous
return type ... when lambda expression has unspecified explicit return
type" even after diagnosing InitListExprs with "cannot deduce lambda
return type from initializer list", which in this case didn't show up
because it accidentally matched with the actual 'void' of the other
return statements.

[aaronpuchert]

Adapt clang-tidy/checkers/cppcoreguidelines-owning-memory.cpp and run clang-format.

[aaronpuchert]

We could also introduce a separate (placeholder) type for initializer lists and perhaps also ParenListExpr if the dependent type seems misleading to you.

CC @JonasToth for the changes to clang-tidy/checkers/cppcoreguidelines-owning-memory.cpp.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-owning-memory.cpp

306

If the warning is looking at this in the original template, we're initializing a gsl::owner<T *> which is type-dependent. So we can't carry out the initialization yet.

Got the impression earlier that the lifetime checks want to “understand” templates which can of course not work because templates can be specialized, overload resolution is only performed on the instantiation, and so on.

clang/test/SemaOpenCLCXX/address-space-references.clcpp

25

Somehow we seem to fail matching the initializer list, but not report that, only on type mismatch later. I think we should either go through with the initialization, then have a short __attribute__((ext_vector_type(2))) and complain that we can't bind the reference to that, or complain when processing the initializer list.

But that's unrelated to this change, void was also not correct.

[aaronpuchert]

Ping @rsmith.

[aaronpuchert]

Ping.

[aaron.ballman]

FWIW, I think this direction seems reasonable to me. Before semantic analysis, we can't know the type, it depends on the context that we haven't analyzed yet. However, I'd ultimately like to hear from @rsmith just in case I am missing some concerns with the changes.

clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-owning-memory.cpp

struct HeapArray {                                          // Ok, since destructor with owner
   HeapArray() : _data(nullptr), size(0) {}                  // Ok
   HeapArray(int size) : _data(new int[size]), size(size) {} // Ok
   HeapArray(int size, T val) {
     _data = new int[size]; // Ok
     size = size;
     for (auto i = 0u; i < size; ++i)
       _data[i] = val; // Ok
   }
   HeapArray(int size, T val, int *problematic) : _data{problematic}, size(size) {} // Bad
-  // CHECK-NOTES: [[@LINE-1]]:50: warning: expected initialization of owner member variable with value of type 'gsl::owner<>'; got 'void'
-  // FIXME: void is incorrect type, probably wrong thing matched
+  // CHECK-NOTES: [[@LINE-1]]:50: warning: expected initialization of owner member variable with value of type 'gsl::owner<>'; got '<dependent type>'
+  // FIXME: <dependent type> is incorrect type, probably wrong thing matched
 
   HeapArray(HeapArray &&other) : _data(other._data), size(other.size) { // Ok
     other._data = nullptr;                                              // Ok
     other.size = 0;
   }
 
   HeapArray<T> &operator=(HeapArray<T> &&other) {
     _data = other._data; // Ok, NOLINT warning here about bad types, why?

clang/include/clang/AST/Expr.h

 /// initialize objects of different types, including
 /// struct/class/union types, arrays, and vectors. For example:
 ///
 /// @code
 /// struct foo x = { 1, { 2, 3 } };
 /// @endcode
 ///
 /// Prior to semantic analysis, an initializer list will represent the
-/// initializer list as written by the user, but will have the
-/// placeholder type "void". This initializer list is called the
+/// initializer list as written by the user, but will have dependent
+/// type as placeholder. This initializer list is called the
 /// syntactic form of the initializer, and may contain C99 designated
 /// initializers (represented as DesignatedInitExprs), initializations
 /// of subobject members without explicit braces, and so on. Clients
 /// interested in the original syntax of the initializer list should
 /// use the syntactic form of the initializer list.
 ///
 /// After semantic analysis, the initializer list will represent the
 /// semantic form of the initializer, where the initializations of all

clang/lib/Sema/SemaExpr.cpp

 Sema::VarArgKind Sema::isValidVarArgType(const QualType &Ty) {
   if (Ty->isIncompleteType()) {
     // C++11 [expr.call]p7:
     //   After these conversions, if the argument does not have arithmetic,
     //   enumeration, pointer, pointer to member, or class type, the program
     //   is ill-formed.
     //
     // Since we've already performed array-to-pointer and function-to-pointer
-    // decay, the only such type in C++ is cv void. This also handles
-    // initializer lists as variadic arguments.
+    // decay, the only such type in C++ is cv void.
     if (Ty->isVoidType())
       return VAK_Invalid;
 
     if (Ty->isObjCObjectType())
       return VAK_Invalid;
     return VAK_Valid;
   }
 
+  // This covers initializer lists.
+  if (Ty->isDependentType())
+    return VAK_Invalid;
+
   if (Ty.isDestructedType() == QualType::DK_nontrivial_c_struct)
     return VAK_Invalid;
 
   if (Ty.isCXX98PODType(Context))
     return VAK_Valid;
 
   // C++11 [expr.call]p7:
   //   Passing a potentially-evaluated argument of class type (Clause 9)
if (InitArgList[I]->getType()->isNonOverloadPlaceholderType()) {
       if (result.isInvalid()) continue;
 
       InitArgList[I] = result.get();
     }
   }
 
   InitListExpr *E = new (Context) InitListExpr(Context, LBraceLoc, InitArgList,
                                                RBraceLoc);
-  E->setType(Context.VoidTy); // FIXME: just a place holder for now.
+  E->setType(Context.DependentTy);
   return E;
 }
 
 /// Do an explicit extend of the given block pointer if we're in ARC.
 void Sema::maybeExtendBlockObject(ExprResult &E) {
   assert(E.get()->getType()->isBlockPointerType());
   assert(E.get()->isPRValue());
 

clang/lib/Sema/SemaInit.cpp

 /// appropriate type, and finish off the InitListExpr formation.
 void
 InitListChecker::FillInEmptyInitializations(const InitializedEntity &Entity,
                                             InitListExpr *ILE,
                                             bool &RequiresSecondPass,
                                             InitListExpr *OuterILE,
                                             unsigned OuterIndex,
                                             bool FillWithNoInit) {
-  assert((ILE->getType() != SemaRef.Context.VoidTy) &&
+  assert((ILE->getType() != SemaRef.Context.DependentTy) &&
          "Should not have void type");
 
   // We don't need to do any checks when just filling NoInitExprs; that can't
   // fail.
   if (FillWithNoInit && VerifyOnly)
     return;
 
   // If this is a nested initializer list, we might have changed its contents

clang/lib/Sema/SemaLambda.cpp

if (Context.getCanonicalFunctionResultType(ReturnType) ==
       auto BlockNullability = CSI.ReturnType->getNullability(Ctx);
       if (BlockNullability &&
           (!RetTyNullability ||
            hasWeakerNullability(*RetTyNullability, *BlockNullability)))
         CSI.ReturnType = ReturnType;
       continue;
     }
 
+    if (isa_and_nonnull<InitListExpr>(RetE))
+      continue; // Should have been diagnosed already, type is bogus.
+
     // FIXME: This is a poor diagnostic for ReturnStmts without expressions.
     // TODO: It's possible that the *first* return is the divergent one.
     Diag(RS->getBeginLoc(),
          diag::err_typecheck_missing_return_type_incompatible)
         << ReturnType << CSI.ReturnType << isa<LambdaScopeInfo>(CSI);
     // Continue iterating so that we keep emitting diagnostics.
   }
 }

clang/test/AST/ast-dump-decl.cpp

namespace testCanonicalTemplate {
 
   // CHECK:     VarTemplateSpecializationDecl 0x{{.+}} <{{.+}}:[[@LINE-21]]:28, col:43> col:43 referenced TestVarTemplate 'const int':'const int' static
   // CHECK-NEXT:`-TemplateArgument type 'int'
   // CHECK-NEXT:  `-BuiltinType 0x{{.+}} 'int'
 
   // CHECK:      VarTemplateDecl 0x{{.+}} parent 0x{{.+}} prev 0x{{.+}} <{{.+}}:[[@LINE-23]]:3, line:[[@LINE-22]]:34> col:14 TestVarTemplate
   // CHECK-NEXT: |-TemplateTypeParmDecl 0x{{.+}} <line:[[@LINE-24]]:12, col:21> col:21 referenced typename depth 0 index 0 T
   // CHECK-NEXT: |-VarDecl 0x{{.+}} parent 0x{{.+}} prev 0x{{.+}} <line:[[@LINE-24]]:3, col:34> col:14 TestVarTemplate 'const T' cinit
-  // CHECK-NEXT: | `-InitListExpr 0x{{.+}} <col:32, col:34> 'void'
+  // CHECK-NEXT: | `-InitListExpr 0x{{.+}} <col:32, col:34> '<dependent type>'
   // CHECK-NEXT: |-VarTemplateSpecialization 0x{{.+}} 'TestVarTemplate' 'const int':'const int'
   // CHECK-NEXT: `-VarTemplateSpecialization 0x{{.+}} 'TestVarTemplate' 'const int':'const int'
-    
   // CHECK:      VarTemplateSpecializationDecl 0x{{.+}} parent 0x{{.+}} prev 0x{{.+}} <{{.+}}:[[@LINE-29]]:3, col:34> col:14 referenced TestVarTemplate 'const int':'const int' cinit
   // CHECK-NEXT: |-TemplateArgument type 'int'
   // CHECK-NEXT: | `-BuiltinType 0x{{.+}} 'int'
   // CHECK-NEXT: `-InitListExpr 0x{{.+}} <col:32, col:34> 'int':'int'
 } 
 
 template <class T>
 class TestClassScopeFunctionSpecialization {

clang/test/AST/ast-dump-recovery.cpp

void InvalidInitalizer(int x) {
   // CHECK-NEXT:  `-InitListExpr
   Bar b2 = {1};
   // CHECK:     `-VarDecl {{.*}} b3 'Bar'
   // CHECK-NEXT:  `-RecoveryExpr {{.*}} 'Bar' contains-errors
   // CHECK-NEXT:    `-DeclRefExpr {{.*}} 'x' 'int'
   Bar b3 = Bar(x);
   // CHECK:     `-VarDecl {{.*}} b4 'Bar'
   // CHECK-NEXT:  `-RecoveryExpr {{.*}} 'Bar' contains-errors
-  // CHECK-NEXT:    `-InitListExpr {{.*}} 'void'
+  // CHECK-NEXT:    `-InitListExpr {{.*}} '<dependent type>'
   // CHECK-NEXT:      `-DeclRefExpr {{.*}} 'x' 'int'
   Bar b4 = Bar{x};
   // CHECK:     `-VarDecl {{.*}} b5 'Bar'
   // CHECK-NEXT: `-CXXUnresolvedConstructExpr {{.*}} 'Bar' contains-errors 'Bar'
   // CHECK-NEXT:   `-RecoveryExpr {{.*}} contains-errors
   // CHECK-NEXT:     `-UnresolvedLookupExpr {{.*}} 'invalid'
   Bar b5 = Bar(invalid());
   // CHECK:     `-VarDecl {{.*}} b6 'Bar'

clang/test/AST/ast-dump-stmt-json.cpp

 // CHECK-NEXT:             },
 // CHECK-NEXT:             "end": {
 // CHECK-NEXT:              "offset": 878,
 // CHECK-NEXT:              "col": 18,
 // CHECK-NEXT:              "tokLen": 1
 // CHECK-NEXT:             }
 // CHECK-NEXT:            },
 // CHECK-NEXT:            "type": {
-// CHECK-NEXT:             "qualType": "void"
+// CHECK-NEXT:             "qualType": "<dependent type>"
 // CHECK-NEXT:            },
 // CHECK-NEXT:            "valueCategory": "prvalue"
 // CHECK-NEXT:           }
 // CHECK-NEXT:          ]
 // CHECK-NEXT:         }
 // CHECK-NEXT:        ]
 // CHECK-NEXT:       },
 // CHECK-NEXT:       {

clang/test/SemaOpenCLCXX/address-space-references.clcpp

public:
   void glob(__global const short2 &); //expected-note{{passing argument to parameter here}}
   void nested_list(const short2 (&)[2]);
 };
 
 void foo() {
   bar(1); // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
   C c;
   c.gen({1, 2});
-  c.glob({1, 2}); //expected-error{{binding reference of type 'const __global short2' (vector of 2 'short' values) to value of type 'void' changes address space}}
+  c.glob({1, 2}); //expected-error{{binding reference of type 'const __global short2' (vector of 2 'short' values) to value of type '<dependent type>' changes address space}}
   c.nested_list({{1, 2}, {3, 4}});
 }
 
 // Test addr space conversion with nested pointers
 
 extern void nestptr(int *&); // expected-note {{candidate function not viable: no known conversion from '__global int *__private' to '__generic int *__generic &__private' for 1st argument}}
 extern void nestptr_const(int * const &); // expected-note {{candidate function not viable: cannot pass pointer to address space '__constant' as a pointer to address space '__generic' in 1st argument}}
 int test_nestptr(__global int *glob, __constant int *cons, int* gen) {
   nestptr(glob); // expected-error{{no matching function for call to 'nestptr'}}
   // Addr space conversion first occurs on a temporary.
   nestptr_const(glob);
   // No legal conversion between disjoint addr spaces.
   nestptr_const(cons); // expected-error{{no matching function for call to 'nestptr_const'}}
   return *(*cons ? glob : gen);
 }