[PATCH] D22476: [AST] Make MemberExpr non-dependent according to core issue 224

[Richard Smith via cfe-commits]

rsmith added inline comments.

================
Comment at: lib/AST/Expr.cpp:1413
@@ +1412,3 @@
+    for (auto *EnableIf : Method->specific_attrs<EnableIfAttr>()) {
+      if (EnableIf->getCond()->isTypeDependent())
+        return false;
----------------
You should presumably be checking `isValueDependent` here.

I also don't see any test changes covering the handling of this attribute, and it's not completely clear to me why a dependent enable_if attribute would cause a member to be treated as not being a member of the current instantiation. This should probably be grouped with the check for a member with a dependent type down on line 1458, not as part of the determination of whether we have a member of the current instantiation.

================
Comment at: lib/AST/Expr.cpp:1456
@@ +1455,3 @@
+  // the field is type dependent. (14.6.2.1 [temp.dep.type])
+  if (E->isTypeDependent()
+   && IsMemberOfCurrentInstantiation(base, memberdecl)
----------------
rsmith wrote:
> Please clang-format this.
The `E->isTypeDependent()` test should be in `isMemberOfCurrentInstantiation` -- right now, that function is determining whether we have a member of the current instantiation or a member of a non-dependent type.

================
Comment at: lib/AST/Expr.cpp:1456-1458
@@ +1455,5 @@
+  // the field is type dependent. (14.6.2.1 [temp.dep.type])
+  if (E->isTypeDependent()
+   && IsMemberOfCurrentInstantiation(base, memberdecl)
+   && !memberdecl->getType()->isDependentType()) {
+    assert(!ty->isDependentType());
----------------
Please clang-format this.

================
Comment at: test/CXX/temp/temp.res/temp.dep/temp.dep.expr/p5.cpp:14-15
@@ +13,4 @@
+      i = nullptr;
+      (*this).i = j; // FIXME {{incompatible type}}
+      this->i = j; // FIXME {{incompatible type}}
+      A::i = j; // expected-error {{incompatible type}}
----------------
Do you know why we don't diagnose this?


https://reviews.llvm.org/D22476