[PATCH] [MSVC] Better lookup into dependent bases for unspecified declaration (also fixes http://llvm.org/PR22066)

Reid Kleckner rnk at google.com
Mon Mar 16 13:59:46 PDT 2015


I think this is pretty close to done, but I want to get an opinion from Richard before committing, since he's more familiar with lookup.


================
Comment at: lib/Sema/SemaDecl.cpp:139-142
@@ +138,6 @@
+  FoundNotSpecifiedKind,
+  /// \brief There are no members is found, but class has a base of class
+  /// TemplateTypeParmType and we are allowed to suppose that the specified
+  /// identifier may be a member of this base.
+  SupposeFoundSpecifiedKind,
+  /// \brief Found a member of the expected type.
----------------
OK, this is sort of the case being described:
  template <typename T> struct Foo : T { void f(int a) { IdFromBase(a); } };

I think we need better terminology to talk about how this case differs from this case:
  template <typename T> struct Foo : Bar<T> { ... };

Both of these are classes with dependent bases, but in the second case, we can go searching through the template pattern for Bar to try to see if it has type or non-type members. I think if we can come up with a name for one of these two cases, we can make this code a lot clearer.

We should get Richard's input on this, but here are some ideas:
- Fully dependent / partially dependent
- Dependent base with known pattern / unknown dependent base

So far, I like using the "known / unknown pattern" terminology best, so I might rename this enumerator to NotFoundWithUnknownBase. This indicates that after doing extended lookup into dependent bases with known patterns, no members with this name were found, but we hit an unknown base that we couldn't look through.

================
Comment at: lib/Sema/SemaDecl.cpp:157
@@ +156,3 @@
+/// case.
+static IdNameLookupResult lookupIdNameInDependentClass(
+    Sema &S, const IdentifierInfo &II, SourceLocation NameLoc,
----------------
If we go with this "known / unknown" terminology, this can be `lookupIdInKnownDependentClass` or something.

================
Comment at: lib/Sema/SemaDecl.cpp:170
@@ +169,3 @@
+  }
+  if (FoundDecl == IdNameLookupResult::NotFound) {
+    for (const auto &Base : RD->bases()) {
----------------
This can be an early return:
  if (FoundDecl != IdNameLookupResult::NotFound)
    return FoundDecl;

================
Comment at: lib/Sema/SemaDecl.cpp:195
@@ +194,3 @@
+      }
+      if (BaseRD) {
+        switch (lookupIdNameInDependentClass(
----------------
This can also continue early:
  if (!BaseRD)
    continue;

================
Comment at: lib/Sema/SemaDecl.cpp:220
@@ -196,1 +219,3 @@
+                       const IdentifierInfo &II, bool SupposeExistInBaseParm,
+                       const std::function<bool(NamedDecl *)> &CheckKind) {
   // Lookup in the parent class template context, if any.
----------------
This is still `std::function`

http://reviews.llvm.org/D8100

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list