[MSVC] 'property' with an empty array in array subscript expression.

[ABataev]

Summary

MSVC supports 'property' attribute and allows to apply it to the declaration of an empty array in a class or structure definition.
For example:

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

The above statement indicates that x[] can be used with one or more array indices. In this case, i=p->x[a][b] will be turned into i=p->GetX(a, b), and p->x[a][b] = i will be turned into p->PutX(a, b, i);

[rnk]

I think fundamentally we are doing too much declspec property lowering in Sema. We might want to back up and figure out how to do it in IRGen. Right now we have bugs like this, which are probably more important than new functionality:
https://llvm.org/bugs/show_bug.cgi?id=24132

[ABataev]

In D13336#257628, @rnk wrote:

I think fundamentally we are doing too much declspec property lowering in Sema. We might want to back up and figure out how to do it in IRGen. Right now we have bugs like this, which are probably more important than new functionality:
https://llvm.org/bugs/show_bug.cgi?id=24132

Currently declspec property is represented as a member function call and does not keep info about property itself. Array subscripts exprs add arguments to this member function call and it must be built in Sema, so we could use an existing codegen for CallExprs.

[rnk]

I spent some time reading through the pseudo-object implementation in clang, but I still don't understand it. We should ask John if he can review this.

My gut feeling is that we shouldn't have a global DenseMap in Sema just for this. Should we have some other node, like an ObjCSubscriptRefExpr that describes this instead?

We need *some* AST for the as-written syntactic form. Can you send a dump of the AST for the test?

lib/Sema/SemaExpr.cpp

3945–3946

Will this affect the syntactic form of the expression?

test/CodeGenCXX/ms-property.cpp

32

Can you add a ++ test?

[rjmccall]

I agree with Reid that you should not be adding a DenseMap to Sema for this. Just build a SubscriptExpr for the syntactic form and have it yield an expression of pseudo-object type; or you can make your own AST node for it if that makes things easier.

[ABataev]

Update after review

[rjmccall]

Needs more tests.

1. Dependent template instantiation.
2. Non-dependent template instantiation.
3. Indexes which are themselves pseudo-objects.
4. Templated getters and setters.
5. Non-POD by-value argument types.

include/clang/AST/BuiltinTypes.def

253

Hmm. I don't think you need a new placeholder type. These subscripts are still an ordinary pseudo-object; you can load and store at any point. Instead, you should detect this case by the expression node used, which can either be an ArraySubscriptExpr or a new node if you find it necessary.

lib/Sema/SemaExpr.cpp

3934

I think what you should do here is:

1. Detect whether the base type has pseudo-object type.
2. If so, give the pseudo-object code a chance to work on the expression. In most cases, it will just load; but if the base is an MSPropertyRefExpr of array type, or an ArraySubscriptExpr, it can leave it alone.
3. If the pseudo-object code returned an expression that's still of pseudo-object type, just build an ArraySubscriptExpr of pseudo-object type after you've folded pseudo-objects in the index as well.

4115

Placeholders should never get here.

lib/Sema/SemaPseudoObject.cpp

1458

Don't modify ArgExprs in buildGet or buildSet; you might need to build both, e.g. when building a compound assignment or inc/dec.

Since you need to copy into a new array anyway, you might as well accumulate them index expressions in stack order and then reverse them when adding them to this array.

[ABataev]

Update after review

[rjmccall]

include/clang/AST/ExprCXX.h

689 ↗

(On Diff #38232)

Please add to the comment here that this is a syntactic pseudo-object expression.

lib/Sema/SemaExpr.cpp

3926

Please factor this check so that it only does extra work when the base has placeholder type, and please extract a function to compute IsMSPropertySubscript.

lib/Sema/SemaPseudoObject.cpp

1429

This code will look slightly nicer and be slightly more efficient if you maintain the invariant that parens have been ignored on Base. That is, IgnoreParens here and when assigning in the loop, and then you can remove them from the type-checks.

Oh. You also need to capture the index expressions so that they aren't evaluated multiple times when this expression is used as the l-value of an increment, decrement, or compound-assignment. You then need to un-capture them in the Rebuilder. Please add code-generation tests that check that the index expression is only evaluated once in these cases.

[ABataev]

Update after review

[ABataev]

John, any comments?

[rjmccall]

lib/Sema/SemaPseudoObject.cpp

1622

Hmm. Just like the ObjCSubscriptRefExpr case, this will need to strip the OVEs off the index expressions, or else you'll end up stacking OVEs and asserting in IRGen.

The deeper problem here is that we have too much redundancy in all these places that have to walk over the different pseudo-object expressions. We can remove some of that by simplifying the design of Rebuilder, which is really over-engineered for its task. It should be fine to make it non-templated, merge all of the rebuildSpecific cases into it (and therefore remove all of the subclasses), and give it a callback to invoke at all the capture points in source order.

I think the callback can just be a llvm::function_ref<Expr*(Expr*, unsigned)>. The second parameter is the position of the capture point in source order, so for example in your case the base of the MSPropertyRefExpr will be 0, the index of the innermost MSPropertySubscriptExpr will be 1, and so on. That'll be important for callers that care about which capture point is which.

The various rebuildAndCaptureObject implementations should use a callback that returns the appropriate captured expression for the position.

stripOpaqueValuesFromPseudoObjectRef should use a callback that returns cast<OpaqueValueExpr>(e)->getSourceExpr(); it shouldn't need to split out the different cases at all.

[ABataev]

lib/Sema/SemaPseudoObject.cpp

1622

John, we don't need to strip the OVEs off the index expressions, because index expressions in MSPropertySubscriptExpr are not folded to OVEs. They are folded to OVEs only for CallArgs array. But in rebuilded MSPropertySubscriptExpr only base MSPropertyRefExpr is folded to OVE (we don't need to put indexes in MSPropertySubscriptExpr, because they are not used during codegen and we can keep original expressions). So, the code should work as is.
I reworked Rebuilder just like you said.

[rjmccall]

lib/Sema/SemaPseudoObject.cpp

54

Well, it should be okay to call this on something that doesn't have a base; it just doesn't need to do anything and can just return refExpr.

1634

That's what I'm saying, though: the capture process should be building the syntactic MSPropertySubscriptExpr using the captured OVEs for the index expressions as well as the recursively-captured base, and Rebuilder will then need to undo that.

We want to consistently replace all the captured sub-trees with their OVEs so that clients can easily recover the relationship between the syntactic and semantic forms.

1664

My hope is that you'll be able to get this whole function down to just this last line and won't need to split out any of the cases here.

[ABataev]

Update after review

[rjmccall]

This looks fantastic, thanks for doing all that. Approved with the one minor change.

lib/Sema/SemaPseudoObject.cpp

159

One extremely minor tweak: could you organize this like the rest of the pseudo-object expressions, checking for it above and calling a method to rebuild it?

[ABataev]

John, thanks a lot for the review!
Committed version has a fix according to your last comment.

include/clang/AST/ASTContext.h

public:
   CanQualType OCLImage1dTy, OCLImage1dArrayTy, OCLImage1dBufferTy;
   CanQualType OCLImage2dTy, OCLImage2dArrayTy, OCLImage2dDepthTy;
   CanQualType OCLImage2dArrayDepthTy, OCLImage2dMSAATy, OCLImage2dArrayMSAATy;
   CanQualType OCLImage2dMSAADepthTy, OCLImage2dArrayMSAADepthTy;
   CanQualType OCLImage3dTy;
   CanQualType OCLSamplerTy, OCLEventTy, OCLClkEventTy;
   CanQualType OCLQueueTy, OCLNDRangeTy, OCLReserveIDTy;
   CanQualType OMPArraySectionTy;
+  CanQualType MSPropertySubscriptTy;
 
   // Types for deductions in C++0x [stmt.ranged]'s desugaring. Built on demand.
   mutable QualType AutoDeductTy;     // Deduction against 'auto'.
   mutable QualType AutoRRefDeductTy; // Deduction against 'auto &&'.
 
   // Decl used to help define __builtin_va_list for some targets.
   // The decl is built when constructing 'BuiltinVaListDecl'.
   mutable Decl *VaListTagDecl;

include/clang/AST/BuiltinTypes.def

 // The type of a cast which, in ARC, would normally require a
 // __bridge, but which might be okay depending on the immediate
 // context.
 PLACEHOLDER_TYPE(ARCUnbridgedCast, ARCUnbridgedCastTy)
 
 // A placeholder type for OpenMP array sections.
 PLACEHOLDER_TYPE(OMPArraySection, OMPArraySectionTy)
 
+// A placeholder type for MS property subscript expression.
+PLACEHOLDER_TYPE(MSPropertySubscript, MSPropertySubscriptTy)
+
 #ifdef LAST_BUILTIN_TYPE
-LAST_BUILTIN_TYPE(OMPArraySection)
+LAST_BUILTIN_TYPE(MSPropertySubscript)
 #undef LAST_BUILTIN_TYPE
 #endif
 
 #undef SHARED_SINGLETON_TYPE
 #undef PLACEHOLDER_TYPE
 #undef FLOATING_TYPE
 #undef SIGNED_TYPE
 #undef UNSIGNED_TYPE
 #undef BUILTIN_TYPE

include/clang/Serialization/ASTBitCodes.h

enum PredefinedTypeIDs {
       PREDEF_TYPE_SAMPLER_ID    = 51,
       /// \brief OpenCL queue type.
       PREDEF_TYPE_QUEUE_ID      = 52,
       /// \brief OpenCL ndrange type.
       PREDEF_TYPE_NDRANGE_ID    = 53,
       /// \brief OpenCL reserve_id type.
       PREDEF_TYPE_RESERVE_ID_ID = 54,
       /// \brief The placeholder type for OpenMP array section.
-      PREDEF_TYPE_OMP_ARRAY_SECTION = 55
+      PREDEF_TYPE_OMP_ARRAY_SECTION = 55,
+      /// \brief The placeholder type for MS declspec property subscript pseudo
+      /// expression.
+      PREDEF_TYPE_MS_PROPERTY_SUBSCRIPT = 56,
     };
 
     /// \brief The number of predefined type IDs that are reserved for
     /// the PREDEF_TYPE_* constants.
     ///
     /// Type IDs for non-predefined types will start at
     /// NUM_PREDEF_TYPE_IDs.
     const unsigned NUM_PREDEF_TYPE_IDS = 100;

lib/AST/ASTContext.cpp

void ASTContext::InitBuiltinTypes(const TargetInfo &Target,
 
   // Placeholder type for builtin functions.
   InitBuiltinType(BuiltinFnTy,  BuiltinType::BuiltinFn);
 
   // Placeholder type for OMP array sections.
   if (LangOpts.OpenMP)
     InitBuiltinType(OMPArraySectionTy, BuiltinType::OMPArraySection);
 
+  if(LangOpts.MSVCCompat)
+    InitBuiltinType(MSPropertySubscriptTy, BuiltinType::MSPropertySubscript);
+
   // C99 6.2.5p11.
   FloatComplexTy      = getComplexType(FloatTy);
   DoubleComplexTy     = getComplexType(DoubleTy);
   LongDoubleComplexTy = getComplexType(LongDoubleTy);
 
   // Builtin types for 'id', 'Class', and 'SEL'.
   InitBuiltinType(ObjCBuiltinIdTy, BuiltinType::ObjCId);
   InitBuiltinType(ObjCBuiltinClassTy, BuiltinType::ObjCClass);

lib/AST/Type.cpp

StringRef BuiltinType::getName(const PrintingPolicy &Policy) const {
   case OCLQueue:
     return "queue_t";
   case OCLNDRange:
     return "event_t";
   case OCLReserveID:
     return "reserve_id_t";
   case OMPArraySection:
     return "<OpenMP array section type>";
+  case MSPropertySubscript:
+    return "<MS declspec property subscript type>";
   }
 
   llvm_unreachable("Invalid builtin type.");
 }
 
 QualType QualType::getNonLValueExprType(const ASTContext &Context) const {
   if (const ReferenceType *RefType = getTypePtr()->getAs<ReferenceType>())
     return RefType->getPointeeType();
#include "clang/AST/BuiltinTypes.def"
     case BuiltinType::OCLEvent:
     case BuiltinType::OCLClkEvent:
     case BuiltinType::OCLQueue:
     case BuiltinType::OCLNDRange:
     case BuiltinType::OCLReserveID:
     case BuiltinType::BuiltinFn:
     case BuiltinType::NullPtr:
     case BuiltinType::OMPArraySection:
+    case BuiltinType::MSPropertySubscript:
       return false;
     }
 
   // Non-pointer types.
   case Type::Complex:
   case Type::LValueReference:
   case Type::RValueReference:
   case Type::ConstantArray:

lib/AST/TypeLoc.cpp

TypeSpecifierType BuiltinTypeLoc::getWrittenTypeSpec() const {
   case BuiltinType::OCLSampler:
   case BuiltinType::OCLEvent:
   case BuiltinType::OCLClkEvent:
   case BuiltinType::OCLQueue:
   case BuiltinType::OCLNDRange:
   case BuiltinType::OCLReserveID:
   case BuiltinType::BuiltinFn:
   case BuiltinType::OMPArraySection:
+  case BuiltinType::MSPropertySubscript:
     return TST_unspecified;
   }
 
   llvm_unreachable("Invalid BuiltinType Kind!");
 }
 
 TypeLoc TypeLoc::IgnoreParensImpl(TypeLoc TL) {
   while (ParenTypeLoc PTL = TL.getAs<ParenTypeLoc>())

lib/Sema/SemaExpr.cpp

if (isa<ParenListExpr>(base)) {
     base = result.get();
   }
 
   // Handle any non-overload placeholder types in the base and index
   // expressions.  We can't handle overloads here because the other
   // operand might be an overloadable type, in which case the overload
   // resolution for the operator overload should get the first crack
   // at the overload.
-  if (base->getType()->isNonOverloadPlaceholderType()) {
+  // MSDN, property (C++)
+  // https://msdn.microsoft.com/en-us/library/yhfk0thd(v=vs.120).aspx
+  // This attribute can also be used in the declaration of an empty array in a
+  // class or structure definition. For example:
+  // __declspec(property(get=GetX, put=PutX)) int x[];
+  // The above statement indicates that x[] can be used with one or more array
+  // indices. In this case, i=p->x[a][b] will be turned into i=p->GetX(a, b),
+  // and p->x[a][b] = i will be turned into p->PutX(a, b, i);
+  auto *MSProp = dyn_cast<MSPropertyRefExpr>(base->IgnoreParens());
+  bool IsMSPropertySubscript =
+      (MSProp && MSProp->getPropertyDecl()->getType()->isArrayType()) ||
+      base->getType()->isSpecificPlaceholderType(
+          BuiltinType::MSPropertySubscript);
+  if (!IsMSPropertySubscript &&
+      base->getType()->isNonOverloadPlaceholderType()) {
     ExprResult result = CheckPlaceholderExpr(base);
     if (result.isInvalid()) return ExprError();
     base = result.get();
   }
   if (idx->getType()->isNonOverloadPlaceholderType()) {
     ExprResult result = CheckPlaceholderExpr(idx);
     if (result.isInvalid()) return ExprError();
     idx = result.get();
   }
 
   // Build an unanalyzed expression if either operand is type-dependent.
   if (getLangOpts().CPlusPlus &&
       (base->isTypeDependent() || idx->isTypeDependent())) {
     return new (Context) ArraySubscriptExpr(base, idx, Context.DependentTy,
                                             VK_LValue, OK_Ordinary, rbLoc);
   }
 
   // Use C++ overloaded-operator rules if either operand has record
   // type.  The spec says to do this if either type is *overloadable*,
   // but enum types can't declare subscript operators or conversion

 
 ExprResult
 Sema::CreateBuiltinArraySubscriptExpr(Expr *Base, SourceLocation LLoc,
                                       Expr *Idx, SourceLocation RLoc) {
   Expr *LHSExp = Base;
   Expr *RHSExp = Idx;
 
   // Perform default conversions.
-  if (!LHSExp->getType()->getAs<VectorType>()) {
+  auto *MSProp = dyn_cast<MSPropertyRefExpr>(LHSExp->IgnoreParens());
+  bool IsMSPropertySubscript =
+      (MSProp && MSProp->getPropertyDecl()->getType()->isArrayType()) ||
+      LHSExp->getType()->isSpecificPlaceholderType(
+          BuiltinType::MSPropertySubscript);
+  if (!LHSExp->getType()->getAs<VectorType>() && !IsMSPropertySubscript) {
     ExprResult Result = DefaultFunctionArrayLvalueConversion(LHSExp);
     if (Result.isInvalid())
       return ExprError();
     LHSExp = Result.get();
   }
   ExprResult Result = DefaultFunctionArrayLvalueConversion(RHSExp);
   if (Result.isInvalid())
     return ExprError();
   RHSExp = Result.get();
 
   QualType LHSTy = LHSExp->getType(), RHSTy = RHSExp->getType();
   ExprValueKind VK = VK_LValue;
   ExprObjectKind OK = OK_Ordinary;
 
   // C99 6.5.2.1p2: the expression e1[e2] is by definition precisely equivalent
   // to the expression *((e1)+(e2)). This means the array "Base" may actually be
   // in the subscript position. As a result, we need to derive the array base
   // and index from the expression types.
   Expr *BaseExpr, *IndexExpr;
   QualType ResultType;
   if (LHSTy->isDependentType() || RHSTy->isDependentType()) {
     BaseExpr = LHSExp;
     IndexExpr = RHSExp;
     ResultType = Context.DependentTy;
+  } else if (IsMSPropertySubscript) {
+    BaseExpr = LHSExp;
+    IndexExpr = RHSExp;
+    ResultType = Context.MSPropertySubscriptTy;
   } else if (const PointerType *PTy = LHSTy->getAs<PointerType>()) {
     BaseExpr = LHSExp;
     IndexExpr = RHSExp;
     ResultType = PTy->getPointeeType();
   } else if (const ObjCObjectPointerType *PTy =
                LHSTy->getAs<ObjCObjectPointerType>()) {
     BaseExpr = LHSExp;
     IndexExpr = RHSExp;
case BuiltinType::Overload:
     return false;
 
   // Unbridged casts in ARC can be handled in some call positions and
   // should be left in place.
   case BuiltinType::ARCUnbridgedCast:
     return false;
 
   // Pseudo-objects should be converted as soon as possible.
+  case BuiltinType::MSPropertySubscript:
   case BuiltinType::PseudoObject:
     return true;
 
   // The debugger mode could theoretically but currently does not try
   // to resolve unknown-typed arguments based on known parameter types.
   case BuiltinType::UnknownAny:
     return true;
 
if (isa<CXXPseudoDestructorExpr>(Fn)) {
           << FixItHint::CreateRemoval(
                                     SourceRange(ArgExprs[0]->getLocStart(),
                                                 ArgExprs.back()->getLocEnd()));
       }
 
       return new (Context)
           CallExpr(Context, Fn, None, Context.VoidTy, VK_RValue, RParenLoc);
     }
-    if (Fn->getType() == Context.PseudoObjectTy) {
+    if (Fn->getType() == Context.PseudoObjectTy ||
+        Fn->getType() == Context.MSPropertySubscriptTy) {
       ExprResult result = CheckPlaceholderExpr(Fn);
       if (result.isInvalid()) return ExprError();
       Fn = result.get();
     }
 
     // Determine whether this is a dependent call inside a C++ template,
     // in which case we won't do any semantic analysis now.
     // FIXME: Will need to cache the results of name lookup (including ADL) in
static void DiagnoseConstAssignment(Sema &S, const Expr *E,
 
   // Can't determine a more specific message, so display the generic error.
   S.Diag(Loc, diag::err_typecheck_assign_const) << ExprRange << ConstUnknown;
 }
 
 /// CheckForModifiableLvalue - Verify that E is a modifiable lvalue.  If not,
 /// emit an error and return true.  If so, return false.
 static bool CheckForModifiableLvalue(Expr *E, SourceLocation Loc, Sema &S) {
-  assert(!E->hasPlaceholderType(BuiltinType::PseudoObject));
+  assert(!E->hasPlaceholderType(BuiltinType::PseudoObject) &&
+         !E->hasPlaceholderType(BuiltinType::MSPropertySubscript));
   SourceLocation OrigLoc = Loc;
   Expr::isModifiableLvalueResult IsLV = E->isModifiableLvalue(S.Context,
                                                               &Loc);
   if (IsLV == Expr::MLV_ClassTemporary && IsReadonlyMessage(E, S))
     IsLV = Expr::MLV_InvalidMessageExpression;
   if (IsLV == Expr::MLV_Valid)
     return false;
 
if (RL && RR && RL->getDecl() == RR->getDecl())
       Sema.Diag(Loc, diag::warn_identity_field_assign) << 1;
   }
 }
 
 // C99 6.5.16.1
 QualType Sema::CheckAssignmentOperands(Expr *LHSExpr, ExprResult &RHS,
                                        SourceLocation Loc,
                                        QualType CompoundType) {
-  assert(!LHSExpr->hasPlaceholderType(BuiltinType::PseudoObject));
+  assert(!LHSExpr->hasPlaceholderType(BuiltinType::PseudoObject) &&
+         !LHSExpr->hasPlaceholderType(BuiltinType::MSPropertySubscript));
 
   // Verify that LHS is a modifiable lvalue, and emit error if not.
   if (CheckForModifiableLvalue(LHSExpr, Loc, *this))
     return QualType();
 
   QualType LHSType = LHSExpr->getType();
   QualType RHSType = CompoundType.isNull() ? RHS.get()->getType() :
                                              CompoundType;
ExprResult Sema::BuildBinOp(Scope *S, SourceLocation OpLoc,
   // (if the LHS is a pseudo-object), BuildOverloadedBinOp (if
   // both expressions are overloadable or either is type-dependent),
   // or CreateBuiltinBinOp (in any other case).  We also want to get
   // any placeholder types out of the way.
 
   // Handle pseudo-objects in the LHS.
   if (const BuiltinType *pty = LHSExpr->getType()->getAsPlaceholderType()) {
     // Assignments with a pseudo-object l-value need special analysis.
-    if (pty->getKind() == BuiltinType::PseudoObject &&
+    if ((pty->getKind() == BuiltinType::PseudoObject ||
+         pty->getKind() == BuiltinType::MSPropertySubscript) &&
         BinaryOperator::isAssignmentOp(Opc))
       return checkPseudoObjectAssignment(S, OpLoc, Opc, LHSExpr, RHSExpr);
 
     // Don't resolve overloads if the other type is overloadable.
     if (pty->getKind() == BuiltinType::Overload) {
       // We can't actually test that if we still have a placeholder,
       // though.  Fortunately, none of the exceptions we see in that
       // code below are valid when the LHS is an overload set.  Note

 }
 
 ExprResult Sema::BuildUnaryOp(Scope *S, SourceLocation OpLoc,
                               UnaryOperatorKind Opc, Expr *Input) {
   // First things first: handle placeholders so that the
   // overloaded-operator check considers the right type.
   if (const BuiltinType *pty = Input->getType()->getAsPlaceholderType()) {
     // Increment and decrement of pseudo-object references.
-    if (pty->getKind() == BuiltinType::PseudoObject &&
+    if ((pty->getKind() == BuiltinType::PseudoObject ||
+         pty->getKind() == BuiltinType::MSPropertySubscript) &&
         UnaryOperator::isIncrementDecrementOp(Opc))
       return checkPseudoObjectIncDec(S, OpLoc, Opc, Input);
 
     // extension is always a builtin operator.
     if (Opc == UO_Extension)
       return CreateBuiltinUnaryOp(OpLoc, Opc, Input);
 
     // & gets special logic for several kinds of placeholder.
case BuiltinType::ARCUnbridgedCast: {
     return realCast;
   }
 
   // Expressions of unknown type.
   case BuiltinType::UnknownAny:
     return diagnoseUnknownAnyExpr(*this, E);
 
   // Pseudo-objects.
+  case BuiltinType::MSPropertySubscript:
   case BuiltinType::PseudoObject:
     return checkPseudoObjectRValue(E);
 
   case BuiltinType::BuiltinFn: {
     // Accept __noop without parens by implicitly converting it to a call expr.
     auto *DRE = dyn_cast<DeclRefExpr>(E->IgnoreParenImpCasts());
     if (DRE) {
       auto *FD = cast<FunctionDecl>(DRE->getDecl());

lib/Sema/SemaPseudoObject.cpp

namespace {
   // Basically just a very focused copy of TreeTransform.
   template <class T> struct Rebuilder {
     Sema &S;
     Rebuilder(Sema &S) : S(S) {}
 
     T &getDerived() { return static_cast<T&>(*this); }
 
     Expr *rebuild(Expr *e) {
       // Fast path: nothing to look through.
       if (typename T::specific_type *specific
             = dyn_cast<typename T::specific_type>(e))
         return getDerived().rebuildSpecific(specific);
 
       // Otherwise, we should look through and rebuild anything that
       // IgnoreParens would.
 
       if (ParenExpr *parens = dyn_cast<ParenExpr>(e)) {
Expr *rebuild(Expr *e) {
                                           rebuiltExpr->getValueKind(),
                                           rebuiltExpr->getObjectKind(),
                                           ce->getRParenLoc(),
                                           ce->isConditionTrue(),
                                           rebuiltExpr->isTypeDependent(),
                                           rebuiltExpr->isValueDependent());
       }
 
+      if (e->getType()->isSpecificPlaceholderType(
+              BuiltinType::MSPropertySubscript)) {
+        auto *ASE = cast<ArraySubscriptExpr>(e);
+        return new (S.Context) ArraySubscriptExpr(
+            rebuild(ASE->getBase()), ASE->getIdx(), ASE->getType(),
+            ASE->getValueKind(), ASE->getObjectKind(), ASE->getRBracketLoc());
+      }
+
+
       llvm_unreachable("bad expression to rebuild!");
     }
   };
 
   struct ObjCPropertyRefRebuilder : Rebuilder<ObjCPropertyRefRebuilder> {
     Expr *NewBase;
     ObjCPropertyRefRebuilder(Sema &S, Expr *newBase)
       : Rebuilder<ObjCPropertyRefRebuilder>(S), NewBase(newBase) {}
Expr *rebuildSpecific(ObjCPropertyRefExpr *refExpr) {
                               refExpr->getType(), refExpr->getValueKind(),
                               refExpr->getObjectKind(), refExpr->getLocation(),
                               NewBase);
       }
       return new (S.Context)
         ObjCPropertyRefExpr(refExpr->getImplicitPropertyGetter(),
                             refExpr->getImplicitPropertySetter(),
                             refExpr->getType(), refExpr->getValueKind(),
                             refExpr->getObjectKind(),refExpr->getLocation(),
                             NewBase);
     }
   };
 
   struct ObjCSubscriptRefRebuilder : Rebuilder<ObjCSubscriptRefRebuilder> {
     Expr *NewBase;
     Expr *NewKeyExpr;
     ObjCSubscriptRefRebuilder(Sema &S, Expr *newBase, Expr *newKeyExpr)
public:
 
    ExprResult buildGet() override;
    ExprResult buildSet(Expr *op, SourceLocation, bool) override;
  };
 
  class MSPropertyOpBuilder : public PseudoOpBuilder {
    MSPropertyRefExpr *RefExpr;
    OpaqueValueExpr *InstanceBase;
+   SmallVector<Expr *, 4> ArgExprs;
 
  public:
    MSPropertyOpBuilder(Sema &S, MSPropertyRefExpr *refExpr) :
      PseudoOpBuilder(S, refExpr->getSourceRange().getBegin()),
      RefExpr(refExpr), InstanceBase(nullptr) {}
+   MSPropertyOpBuilder(Sema &S, Expr *E)
+       : PseudoOpBuilder(S, E->getSourceRange().getBegin()),
+         InstanceBase(nullptr) {
+     assert(E->getType()->isSpecificPlaceholderType(
+         BuiltinType::MSPropertySubscript));
+     auto *Base = E->IgnoreParens();
+     while (Base->getType()->isSpecificPlaceholderType(
+         BuiltinType::MSPropertySubscript)) {
+       auto *ASE = cast<ArraySubscriptExpr>(Base);
+       ArgExprs.insert(ArgExprs.begin(), ASE->getIdx());
+       Base = ASE->getBase()->IgnoreParens();
+     }
+     RefExpr = cast<MSPropertyRefExpr>(Base);
+   }
 
    Expr *rebuildAndCaptureObject(Expr *) override;
    ExprResult buildGet() override;
    ExprResult buildSet(Expr *op, SourceLocation, bool) override;
  };
 }
 
 /// Capture the given expression in an OpaqueValueExpr.

 }
 
 //===----------------------------------------------------------------------===//
 //  MSVC __declspec(property) references
 //===----------------------------------------------------------------------===//
 
 Expr *MSPropertyOpBuilder::rebuildAndCaptureObject(Expr *syntacticBase) {
   InstanceBase = capture(RefExpr->getBaseExpr());
 
   syntacticBase =
     MSPropertyRefRebuilder(S, InstanceBase).rebuild(syntacticBase);
 
   return syntacticBase;
 }
 
 ExprResult MSPropertyOpBuilder::buildGet() {
   if (!RefExpr->getPropertyDecl()->hasGetter()) {
ExprResult GetterExpr =
                               RefExpr->isArrow() ? tok::arrow : tok::period, SS,
                               SourceLocation(), GetterName, nullptr);
   if (GetterExpr.isInvalid()) {
     S.Diag(RefExpr->getMemberLoc(),
            diag::error_cannot_find_suitable_accessor) << 0 /* getter */
       << RefExpr->getPropertyDecl();
     return ExprError();
   }
 
-  MultiExprArg ArgExprs;
   return S.ActOnCallExpr(S.getCurScope(), GetterExpr.get(),
                          RefExpr->getSourceRange().getBegin(), ArgExprs,
                          RefExpr->getSourceRange().getEnd());
 }
 
 ExprResult MSPropertyOpBuilder::buildSet(Expr *op, SourceLocation sl,
                                          bool captureSetValueAsResult) {
   if (!RefExpr->getPropertyDecl()->hasSetter()) {
ExprResult SetterExpr =
                               SourceLocation(), SetterName, nullptr);
   if (SetterExpr.isInvalid()) {
     S.Diag(RefExpr->getMemberLoc(),
            diag::error_cannot_find_suitable_accessor) << 1 /* setter */
       << RefExpr->getPropertyDecl();
     return ExprError();
   }
 
-  SmallVector<Expr*, 1> ArgExprs;
   ArgExprs.push_back(op);
   return S.ActOnCallExpr(S.getCurScope(), SetterExpr.get(),
                          RefExpr->getSourceRange().getBegin(), ArgExprs,
                          op->getSourceRange().getEnd());
 }
 
 //===----------------------------------------------------------------------===//
 //  General Sema routines.
ExprResult Sema::checkPseudoObjectRValue(Expr *E) {
   else if (ObjCSubscriptRefExpr *refExpr
            = dyn_cast<ObjCSubscriptRefExpr>(opaqueRef)) {
     ObjCSubscriptOpBuilder builder(*this, refExpr);
     return builder.buildRValueOperation(E);
   } else if (MSPropertyRefExpr *refExpr
              = dyn_cast<MSPropertyRefExpr>(opaqueRef)) {
     MSPropertyOpBuilder builder(*this, refExpr);
     return builder.buildRValueOperation(E);
+  } else if (E->getType()->isSpecificPlaceholderType(
+                 BuiltinType::MSPropertySubscript)) {
+    MSPropertyOpBuilder builder(*this, E);
+    return builder.buildRValueOperation(E);
   } else {
     llvm_unreachable("unknown pseudo-object kind!");
   }
 }
 
 /// Check an increment or decrement of a pseudo-object expression.
 ExprResult Sema::checkPseudoObjectIncDec(Scope *Sc, SourceLocation opcLoc,
                                          UnaryOperatorKind opcode, Expr *op) {
if (ObjCPropertyRefExpr *refExpr
     return builder.buildIncDecOperation(Sc, opcLoc, opcode, op);
   } else if (isa<ObjCSubscriptRefExpr>(opaqueRef)) {
     Diag(opcLoc, diag::err_illegal_container_subscripting_op);
     return ExprError();
   } else if (MSPropertyRefExpr *refExpr
              = dyn_cast<MSPropertyRefExpr>(opaqueRef)) {
     MSPropertyOpBuilder builder(*this, refExpr);
     return builder.buildIncDecOperation(Sc, opcLoc, opcode, op);
+  } else if (opaqueRef->getType()->isSpecificPlaceholderType(
+                 BuiltinType::MSPropertySubscript)) {
+    MSPropertyOpBuilder builder(*this, opaqueRef);
+    return builder.buildIncDecOperation(Sc, opcLoc, opcode, op);
   } else {
     llvm_unreachable("unknown pseudo-object kind!");
   }
 }
 
 ExprResult Sema::checkPseudoObjectAssignment(Scope *S, SourceLocation opcLoc,
                                              BinaryOperatorKind opcode,
                                              Expr *LHS, Expr *RHS) {
if (ObjCPropertyRefExpr *refExpr
     ObjCPropertyOpBuilder builder(*this, refExpr);
     return builder.buildAssignmentOperation(S, opcLoc, opcode, LHS, RHS);
   } else if (ObjCSubscriptRefExpr *refExpr
              = dyn_cast<ObjCSubscriptRefExpr>(opaqueRef)) {
     ObjCSubscriptOpBuilder builder(*this, refExpr);
     return builder.buildAssignmentOperation(S, opcLoc, opcode, LHS, RHS);
   } else if (MSPropertyRefExpr *refExpr
              = dyn_cast<MSPropertyRefExpr>(opaqueRef)) {
       MSPropertyOpBuilder builder(*this, refExpr);
       return builder.buildAssignmentOperation(S, opcLoc, opcode, LHS, RHS);
+  } else if (opaqueRef->getType()->isSpecificPlaceholderType(
+                 BuiltinType::MSPropertySubscript)) {
+    MSPropertyOpBuilder builder(*this, opaqueRef);
+    return builder.buildAssignmentOperation(S, opcLoc, opcode, LHS, RHS);
   } else {
     llvm_unreachable("unknown pseudo-object kind!");
   }
 }
 
 /// Given a pseudo-object reference, rebuild it without the opaque
 /// values.  Basically, undo the behavior of rebuildAndCaptureObject.
 /// This should never operate in-place.
if (ObjCPropertyRefExpr *refExpr
     OpaqueValueExpr *baseOVE = cast<OpaqueValueExpr>(refExpr->getBaseExpr());
     OpaqueValueExpr *keyOVE = cast<OpaqueValueExpr>(refExpr->getKeyExpr());
     return ObjCSubscriptRefRebuilder(S, baseOVE->getSourceExpr(), 
                                      keyOVE->getSourceExpr()).rebuild(E);
   } else if (MSPropertyRefExpr *refExpr
              = dyn_cast<MSPropertyRefExpr>(opaqueRef)) {
     OpaqueValueExpr *baseOVE = cast<OpaqueValueExpr>(refExpr->getBaseExpr());
     return MSPropertyRefRebuilder(S, baseOVE->getSourceExpr()).rebuild(E);
+  } else if (opaqueRef->getType()->isSpecificPlaceholderType(
+                 BuiltinType::MSPropertySubscript)) {
+    while (opaqueRef->getType()->isSpecificPlaceholderType(
+        BuiltinType::MSPropertySubscript)) {
+      opaqueRef =
+          cast<ArraySubscriptExpr>(opaqueRef)->getBase()->IgnoreParens();
+    }
+    auto *BaseOVE = cast<OpaqueValueExpr>(
+        cast<MSPropertyRefExpr>(opaqueRef)->getBaseExpr());
+    return MSPropertyRefRebuilder(S, BaseOVE->getSourceExpr()).rebuild(E);
   } else {
     llvm_unreachable("unknown pseudo-object kind!");
   }
 }
 
 /// Given a pseudo-object expression, recreate what it looks like
 /// syntactically without the attendant OpaqueValueExprs.
 ///
 /// This is a hack which should be removed when TreeTransform is
 /// capable of rebuilding a tree without stripping implicit
 /// operations.
 Expr *Sema::recreateSyntacticForm(PseudoObjectExpr *E) {
   Expr *syntax = E->getSyntacticForm();
   if (UnaryOperator *uop = dyn_cast<UnaryOperator>(syntax)) {
     Expr *op = stripOpaqueValuesFromPseudoObjectRef(*this, uop->getSubExpr());
     return new (Context) UnaryOperator(op, uop->getOpcode(), uop->getType(),
                                        uop->getValueKind(), uop->getObjectKind(),
                                        uop->getOperatorLoc());
   } else if (CompoundAssignOperator *cop
                = dyn_cast<CompoundAssignOperator>(syntax)) {
Expr *Sema::recreateSyntacticForm(PseudoObjectExpr *E) {
   } else if (BinaryOperator *bop = dyn_cast<BinaryOperator>(syntax)) {
     Expr *lhs = stripOpaqueValuesFromPseudoObjectRef(*this, bop->getLHS());
     Expr *rhs = cast<OpaqueValueExpr>(bop->getRHS())->getSourceExpr();
     return new (Context) BinaryOperator(lhs, rhs, bop->getOpcode(),
                                         bop->getType(), bop->getValueKind(),
                                         bop->getObjectKind(),
                                         bop->getOperatorLoc(), false);
   } else {
-    assert(syntax->hasPlaceholderType(BuiltinType::PseudoObject));
+    assert(syntax->hasPlaceholderType(BuiltinType::PseudoObject) ||
+           syntax->hasPlaceholderType(BuiltinType::MSPropertySubscript));
     return stripOpaqueValuesFromPseudoObjectRef(*this, syntax);
   }
 }

lib/Serialization/ASTCommon.cpp

case BuiltinType::OCLReserveID:
     ID = PREDEF_TYPE_RESERVE_ID_ID;
     break;
   case BuiltinType::BuiltinFn:
     ID = PREDEF_TYPE_BUILTIN_FN;
     break;
   case BuiltinType::OMPArraySection:
     ID = PREDEF_TYPE_OMP_ARRAY_SECTION;
     break;
+  case BuiltinType::MSPropertySubscript:
+    ID = PREDEF_TYPE_MS_PROPERTY_SUBSCRIPT;
+    break;
   }
 
   return TypeIdx(ID);
 }
 
 unsigned serialization::ComputeHash(Selector Sel) {
   unsigned N = Sel.getNumArgs();
   if (N == 0)

lib/Serialization/ASTReader.cpp

if (Index < NUM_PREDEF_TYPE_IDS) {
 
     case PREDEF_TYPE_BUILTIN_FN:
       T = Context.BuiltinFnTy;
       break;
 
     case PREDEF_TYPE_OMP_ARRAY_SECTION:
       T = Context.OMPArraySectionTy;
       break;
+
+    case PREDEF_TYPE_MS_PROPERTY_SUBSCRIPT:
+      T = Context.MSPropertySubscriptTy;
+      break;
     }
 
     assert(!T.isNull() && "Unknown predefined type");
     return T.withFastQualifiers(FastQuals);
   }
 
   Index -= NUM_PREDEF_TYPE_IDS;
   assert(Index < TypesLoaded.size() && "Type index out-of-range");

test/CodeGenCXX/ms-property.cpp

 // RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s
+// RUN: %clang_cc1 -emit-llvm -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -o - | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
 
 class Test1 {
 private:
   int x_;
 
 public:
   Test1(int x) : x_(x) {}
   __declspec(property(get = get_x)) int X;
   int get_x() const { return x_; }
   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; }
+};
+
+template <typename T>
+class St {
+public:
+  __declspec(property(get=GetX,put=PutX)) T x[];
+  T GetX(T i, T j) { return i+j; }
+  void PutX(T i, T j, T k) { j = i = k; }
+};
+
 // CHECK-LABEL: main
 int main(int argc, char **argv) {
+  S *p1 = 0;
+  St<float> *p2 = 0;
+  // CHECK: call i32 @"\01?GetX@S@@QEAAHHH@Z"(%class.S* %{{.+}}, i32 223, i32 11)
+  int j = p1->x[223][11];
+  // CHECK: [[J:%.+]] = load i32, i32* %
+  // CHECK-NEXT: call void @"\01?PutX@S@@QEAAXHHH@Z"(%class.S* %{{.+}}, i32 23, i32 1, i32 [[J]])
+  p1->x[23][1] = j;
+  // CHECK: call float @"\01?GetX@?$St@M@@QEAAMMM@Z"(%class.St* %{{.+}}, float 2.230000e+02, float 1.100000e+01)
+  float j1 = p2->x[223][11];
+  // CHECK: [[J1:%.+]] = load float, float* %
+  // CHECK-NEXT: call void @"\01?PutX@?$St@M@@QEAAXMMM@Z"(%class.St* %{{.+}}, float 2.300000e+01, float 1.000000e+00, float [[J1]])
+  p2->x[23][1] = j1;
+  // CHECK: [[GET:%.+]] = call float @"\01?GetX@?$St@M@@QEAAMMM@Z"(%class.St* %6, float 2.300000e+01, float 1.000000e+00)
+  // CHECK-NEXT: [[INC:%.+]] = fadd float [[GET]], 1.000000e+00
+  // CHECK-NEXT: call void @"\01?PutX@?$St@M@@QEAAXMMM@Z"(%class.St* %6, float 2.300000e+01, float 1.000000e+00, float [[INC]])
+  ++p2->x[23][1];
   // CHECK: [[CALL:%.+]] = call %class.Test1* @"\01?GetTest1@Test1@@SAPEAV1@XZ"()
   // CHECK-NEXT: call i32 @"\01?get_x@Test1@@QEBAHXZ"(%class.Test1* [[CALL]])
   return Test1::GetTest1()->X;
 }
+
+#endif //HEADER

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}}
+};
+
+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}}
+  void PutX(T i, T j, T k) { j = i = k; }  // expected-note 2 {{'PutX' declared here}}
+  ~St() {
+    x[1] = 0; // expected-error {{too few arguments to function call, expected 3, have 2}}
+    x[2][3] = 4;
+    ++x[2][3];
+    x[1][2] = x[3][4][5]; // expected-error {{too many arguments to function call, expected 2, have 3}}
+  }
+};
+
+// 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}}
+  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}}
+  return ++(((p2->x)[23])); // expected-error {{too few arguments to function call, expected 2, have 1}}
+}

test/SemaCXX/ms-property.cpp

+// RUN: %clang_cc1 -ast-print -verify -triple=x86_64-pc-win32 -fms-compatibility %s -o - | FileCheck %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t %s
+// RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t -verify %s -ast-print -o - | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+class Test1 {
+private:
+  int x_;
+
+public:
+  Test1(int x) : x_(x) {}
+  __declspec(property(get = get_x)) int X;
+  int get_x() const { return x_; }
+  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; }
+};
+
+template <typename T>
+class St {
+public:
+  __declspec(property(get=GetX,put=PutX)) T x[];
+  T GetX(T i, T j) { return i+j; }
+  void PutX(T i, T j, T k) { j = i = k; }
+  ~St() { x[0][0] = x[1][1]; }
+};
+
+// CHECK: this->x[0][0] = this->x[1][1];
+// CHECK: this->x[0][0] = this->x[1][1];
+
+// 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: 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: ++(((p2->x)[23])[1]);
+  ++(((p2->x)[23])[1]);
+  // CHECK-NEXT: return Test1::GetTest1()->X;
+  return Test1::GetTest1()->X;
+}
+#endif // HEADER