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
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
