[MSVC] Allow declaration of multi-dim 'property' array fields

[mikerice]

Summary

MSVC allows declaration of multi-dim arrays like this:

__declspec(property(get=GetX, put=PutX)) int x[][][];

This syntax can appear in generated typelib headers.

Currently clang errors on declarators like this since it forms an array
type of incomplete array. Rather than try to handle such a type, ignore
adjacent empty chunks so this is treated as if there was only one empty
array chunk (i.e. int x[]).

The functionality to handle multi-dim subscripts of property fields
already works, but only if declared as a single-dim array.

[erichkeane]

LGTM, this does what I'd expect it to do. Thanks!

clang/test/SemaCXX/ms-property-error.cpp

9

These are exactly the cases I despise where we split notes/diags. As discussed externally, that diagnostic message is awful, but I think outside of the scope of this patch.

[erichkeane]

Ah, needs a release note however!

[aaron.ballman]

Changes LGTM but please add a release note about the new functionality when landing.

[mikerice]

57caadc57a30f2279099e5b86bb555b4aab621ce

clang/include/clang/Sema/ParsedAttr.h

public:
   }
 
   bool hasAttribute(ParsedAttr::Kind K) const {
     return llvm::any_of(AttrList, [K](const ParsedAttr *AL) {
       return AL->getParsedKind() == K;
     });
   }
 
+  const ParsedAttr *getMSPropertyAttr() const {
+    auto It = llvm::find_if(AttrList, [](const ParsedAttr *AL) {
+      return AL->isDeclspecPropertyAttribute();
+    });
+    if (It != AttrList.end())
+      return *It;
+    return nullptr;
+  }
+  bool hasMSPropertyAttr() const { return getMSPropertyAttr(); }
+
 private:
   VecTy AttrList;
 };
 
 /// ParsedAttributes - A collection of parsed attributes.  Currently
 /// we don't differentiate between the various attribute syntaxes,
 /// which is basically silly.
 ///

clang/lib/Sema/SemaDeclCXX.cpp

static bool InitializationHasSideEffects(const FieldDecl &FD) {
   // FIXME: Destruction of ObjC lifetime types has side-effects.
   if (const CXXRecordDecl *RD = T->getAsCXXRecordDecl())
     return !RD->isCompleteDefinition() ||
            !RD->hasTrivialDefaultConstructor() ||
            !RD->hasTrivialDestructor();
   return false;
 }
 
-static const ParsedAttr *getMSPropertyAttr(const ParsedAttributesView &list) {
-  ParsedAttributesView::const_iterator Itr =
-      llvm::find_if(list, [](const ParsedAttr &AL) {
-        return AL.isDeclspecPropertyAttribute();
-      });
-  if (Itr != list.end())
-    return &*Itr;
-  return nullptr;
-}
-
 // Check if there is a field shadowing.
 void Sema::CheckShadowInheritedFields(const SourceLocation &Loc,
                                       DeclarationName FieldName,
                                       const CXXRecordDecl *RD,
                                       bool DeclIsField) {
   if (Diags.isIgnored(diag::warn_shadow_field, Loc))
     return;
 
Sema::ActOnCXXMemberDeclarator(Scope *S, AccessSpecifier AS, Declarator &D,
 
   Expr *BitWidth = static_cast<Expr*>(BW);
 
   assert(isa<CXXRecordDecl>(CurContext));
   assert(!DS.isFriendSpecified());
 
   bool isFunc = D.isDeclarationOfFunction();
   const ParsedAttr *MSPropertyAttr =
-      getMSPropertyAttr(D.getDeclSpec().getAttributes());
+      D.getDeclSpec().getAttributes().getMSPropertyAttr();
 
   if (cast<CXXRecordDecl>(CurContext)->isInterface()) {
     // The Microsoft extension __interface only permits public member functions
     // and prohibits constructors, destructors, operators, non-public member
     // functions, static methods and data members.
     unsigned InvalidDecl;
     bool ShowDeclName = true;
     if (!isFunc &&

clang/lib/Sema/SemaType.cpp

case DeclaratorChunk::Array: {
       if (LangOpts.CPlusPlus && S.CheckDistantExceptionSpec(T)) {
         S.Diag(D.getIdentifierLoc(), diag::err_distant_exception_spec);
         D.setInvalidType(true);
         // Build the type anyway.
       }
       DeclaratorChunk::ArrayTypeInfo &ATI = DeclType.Arr;
       Expr *ArraySize = static_cast<Expr*>(ATI.NumElts);
       ArrayType::ArraySizeModifier ASM;
+
+      // Microsoft property fields can have multiple sizeless array chunks
+      // (i.e. int x[][][]). Skip all of these except one to avoid creating
+      // bad incomplete array types.
+      if (chunkIndex != 0 && !ArraySize &&
+          D.getDeclSpec().getAttributes().hasMSPropertyAttr()) {
+        // This is a sizeless chunk. If the next is also, skip this one.
+        DeclaratorChunk &NextDeclType = D.getTypeObject(chunkIndex - 1);
+        if (NextDeclType.Kind == DeclaratorChunk::Array &&
+            !NextDeclType.Arr.NumElts)
+          break;
+      }
+
       if (ATI.isStar)
         ASM = ArrayType::Star;
       else if (ATI.hasStatic)
         ASM = ArrayType::Static;
       else
         ASM = ArrayType::Normal;
       if (ASM == ArrayType::Star && !D.isPrototypeContext()) {
         // FIXME: This check isn't quite right: it allows star in prototypes
GetTypeSourceInfoForDeclarator(TypeProcessingState &State,
 
   // Handle parameter packs whose type is a pack expansion.
   if (isa<PackExpansionType>(T)) {
     CurrTL.castAs<PackExpansionTypeLoc>().setEllipsisLoc(D.getEllipsisLoc());
     CurrTL = CurrTL.getNextTypeLoc().getUnqualifiedLoc();
   }
 
   for (unsigned i = 0, e = D.getNumTypeObjects(); i != e; ++i) {
+    // Microsoft property fields can have multiple sizeless array chunks
+    // (i.e. int x[][][]). Don't create more than one level of incomplete array.
+    if (CurrTL.getTypeLocClass() == TypeLoc::IncompleteArray && e != 1 &&
+        D.getDeclSpec().getAttributes().hasMSPropertyAttr())
+      continue;
+
     // An AtomicTypeLoc might be produced by an atomic qualifier in this
     // declarator chunk.
     if (AtomicTypeLoc ATL = CurrTL.getAs<AtomicTypeLoc>()) {
       fillAtomicQualLoc(ATL, D.getTypeObject(i));
       CurrTL = ATL.getValueLoc().getUnqualifiedLoc();
     }
 
     bool HasDesugaredTypeLoc = true;

clang/test/CodeGenCXX/ms-property.cpp

public:
   static Test1 *GetTest1() { return new Test1(10); }
 };
 
 class S {
 public:
   __declspec(property(get=GetX,put=PutX)) int x[];
   int GetX(int i, int j) { return i+j; }
   void PutX(int i, int j, int k) { j = i = k; }
+  __declspec(property(get=GetY,put=PutY)) int y[][];
+  int GetY(int i, int j) { return i+j; }
+  void PutY(int i, int j, int k) { j = i = k; }
+  __declspec(property(get=GetZ,put=PutZ)) int z[][][];
+  int GetZ(int i, int j, int k) { return i+j+k; }
+  void PutZ(int i, int j, int k, int v) { j = i = k = v; }
 };
 
 template <typename T>
 class St {
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }
   T GetX() { return 0; }
int main(int argc, char **argv) {
   Test1 t(argc);
   S *p1 = 0;
   St<float> *p2 = 0;
   // CHECK: call noundef i32 @"?GetX@S@@QEAAHHH@Z"(%class.S* {{[^,]*}} %{{.+}}, i32 noundef 223, i32 noundef 11)
   int j = p1->x[223][11];
   // CHECK: [[J:%.+]] = load i32, i32* %
   // CHECK-NEXT: call void @"?PutX@S@@QEAAXHHH@Z"(%class.S* {{[^,]*}} %{{.+}}, i32 noundef 23, i32 noundef 1, i32 noundef [[J]])
   p1->x[23][1] = j;
+  // CHECK: call noundef i32 @"?GetY@S@@QEAAHHH@Z"(%class.S* {{[^,]*}} %{{.+}}, i32 noundef 123, i32 noundef 22)
+  int k = p1->y[123][22];
+  // CHECK: [[K:%.+]] = load i32, i32* %
+  // CHECK-NEXT: call void @"?PutY@S@@QEAAXHHH@Z"(%class.S* {{[^,]*}} %{{.+}}, i32 noundef 16, i32 noundef 2, i32 noundef [[K]])
+  p1->y[16][2] = k;
+  // CHECK: call noundef i32 @"?GetZ@S@@QEAAHHHH@Z"(%class.S* {{[^,]*}} %{{.+}}, i32 noundef 123, i32 noundef 22, i32 noundef 44)
+  k = p1->z[123][22][44];
+  // CHECK: [[K:%.+]] = load i32, i32* %
+  // CHECK-NEXT: call void @"?PutZ@S@@QEAAXHHHH@Z"(%class.S* {{[^,]*}} %{{.+}}, i32 noundef 16, i32 noundef 2, i32 noundef 32, i32 noundef [[K]])
+  p1->z[16][2][32] = k;
   // CHECK: call noundef float @"?GetX@?$St@M@@QEAAMMM@Z"(%class.St* {{[^,]*}} %{{.+}}, float noundef 2.230000e+02, float noundef 1.100000e+01)
   float j1 = p2->x[223][11];
   // CHECK: [[J1:%.+]] = load float, float* %
   // CHECK-NEXT: [[CALL:%.+]] = call noundef float @"?PutX@?$St@M@@QEAAMMMM@Z"(%class.St* {{[^,]*}} %{{.+}}, float noundef 2.300000e+01, float noundef 1.000000e+00, float noundef [[J1]])
   // CHECK-NEXT: [[CONV:%.+]] = fptosi float [[CALL]] to i32
   // CHECK-NEXT: store i32 [[CONV]], i32*
   argc = p2->x[23][1] = j1;
   // CHECK: [[IDX:%.+]] = call noundef i32 @"?idx@@YAHXZ"()

clang/test/SemaCXX/ms-property-error.cpp

 // RUN: %clang_cc1 -verify -fms-compatibility %s -fsyntax-only -o -
 
 class S {
 public:
   __declspec(property(get=GetX,put=PutX)) int x[];
   int GetX(int i, int j) { return i+j; } // expected-note {{'GetX' declared here}}
   void PutX(int i, int j, int k) { j = i = k; } // expected-note {{'PutX' declared here}}
+  __declspec(property(get=GetY,put=PutY)) int y[][][];
+  int GetY(int i, int j) { return i+j; } // expected-note {{'GetY' declared here}}
+  void PutY(int i, int j, int k) { j = i = k; } // expected-note {{'PutY' declared here}}
 };
 
 char *ptr;
 template <typename T>
 class St {
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; } // expected-note 3 {{'GetX' declared here}}

 
 // CHECK-LABEL: main
 int main(int argc, char **argv) {
   S *p1 = 0;
   St<float> *p2 = 0;
   St<int> a; // expected-note {{in instantiation of member function 'St<int>::~St' requested here}}
   int j = (p1->x)[223][11][2]; // expected-error {{too many arguments to function call, expected 2, have 3}}
   (p1->x[23]) = argc; // expected-error {{too few arguments to function call, expected 3, have 2}}
+  int k = (p1->y)[223][11][2][4]; // expected-error {{too many arguments to function call, expected 2, have 4}}
+  (p1->y[23]) = argc; // expected-error {{too few arguments to function call, expected 3, have 2}}
   float j1 = (p2->x); // expected-error {{too few arguments to function call, expected 2, have 0}}
   ((p2->x)[23])[1][2] = *argv; // expected-error {{too many arguments to function call, expected 3, have 4}}
   argv = p2->x[11][22] = argc; // expected-error {{assigning to 'char **' from incompatible type 'float'}}
   return ++(((p2->x)[23])); // expected-error {{too few arguments to function call, expected 2, have 1}}
 }

clang/test/SemaCXX/ms-property.cpp

public:
   static Test1 *GetTest1() { return new Test1(10); }
 };
 
 class S {
 public:
   __declspec(property(get=GetX,put=PutX)) int x[];
   int GetX(int i, int j) { return i+j; }
   void PutX(int i, int j, int k) { j = i = k; }
+  __declspec(property(get=GetY,put=PutY)) int y[][];
+  int GetY(int i, int j) { return i+j; }
+  void PutY(int i, int j, int k) { j = i = k; }
+  __declspec(property(get=GetZ,put=PutZ)) int z[][][];
+  int GetZ(int i, int j, int k);
+  void PutZ(int i, int j, int k, int val);
 };
 
 template <typename T>
 class St {
 public:
   __declspec(property(get=GetX,put=PutX)) T x[];
   T GetX(T i, T j) { return i+j; }
   T PutX(T i, T j, T k) { return j = i = k; }
-  ~St() { x[0][0] = x[1][1]; }
+  __declspec(property(get=GetY,put=PutY)) T y[][];
+  T GetY(T i, T j) { return i+j; }
+  T PutY(T i, T j, T k) { return j = i = k; }
+  __declspec(property(get=GetZ,put=PutZ)) T z[][][];
+  T GetZ(T i, T j, T k) { return i+j+k; }
+  T PutZ(T i, T j, T k, T v) { return j = i = k = v; }
+  ~St() { x[0][0] = x[1][1]; y[0][0] = x[1][1]; z[0][1][2] = z[2][1][0]; }
 };
 
 // CHECK: this->x[0][0] = this->x[1][1];
+// CHECK: this->y[0][0] = this->x[1][1];
+// CHECK: this->z[0][1][2] = this->z[2][1][0];
 // CHECK: this->x[0][0] = this->x[1][1];
+// CHECK: this->y[0][0] = this->x[1][1];
+// CHECK: this->z[0][1][2] = this->z[2][1][0];
 
 // CHECK-LABEL: main
 int main(int argc, char **argv) {
   S *p1 = 0;
   St<float> *p2 = 0;
   // CHECK: St<int> a;
   St<int> a;
   // CHECK-NEXT: int j = (p1->x)[223][11];
   int j = (p1->x)[223][11];
   // CHECK-NEXT: (p1->x[23])[1] = j;
   (p1->x[23])[1] = j;
+  // CHECK-NEXT: int k = (p1->y)[223][11];
+  int k = (p1->y)[223][11];
+  // CHECK-NEXT: (p1->y[23])[1] = k;
+  (p1->y[23])[1] = k;
+  // CHECK-NEXT: int k3 = p1->z[1][2][3];
+  int k3 = p1->z[1][2][3];
+  // CHECK-NEXT: p1->z[0][2][1] = k3;
+  p1->z[0][2][1] = k3;
   // CHECK-NEXT: float j1 = (p2->x[223][11]);
   float j1 = (p2->x[223][11]);
   // CHECK-NEXT: ((p2->x)[23])[1] = j1;
   ((p2->x)[23])[1] = j1;
+  // CHECK-NEXT: float k1 = (p2->y[223][11]);
+  float k1 = (p2->y[223][11]);
+  // CHECK-NEXT: ((p2->y)[23])[1] = k1;
+  ((p2->y)[23])[1] = k1;
   // CHECK-NEXT: ++(((p2->x)[23])[1]);
   ++(((p2->x)[23])[1]);
   // CHECK-NEXT: j1 = ((p2->x)[23])[1] = j1;
   j1 = ((p2->x)[23])[1] = j1;
   // CHECK-NEXT: return Test1::GetTest1()->X;
   return Test1::GetTest1()->X;
 }
 #endif // HEADER