[PATCH] [MSVC] Improved lookup into dependent/non-dependent bases of dependent class

Richard Smith richard at metafoo.co.uk
Wed Feb 4 17:00:36 PST 2015


================
Comment at: lib/AST/Type.cpp:1894-1900
@@ -1893,5 +1893,9 @@
 
 TagDecl *TagType::getDecl() const {
   return getInterestingTagDecl(decl);
 }
 
+TagDecl *TagType::getDecl() {
+  return getInterestingTagDecl(decl);
+}
+
----------------
What's the purpose of this change? We'll call the `const` version for a non-const object without this, which will do the same thing.

================
Comment at: lib/Sema/SemaDecl.cpp:132
@@ +131,3 @@
+namespace {
+enum BasesLookupResult { NotFound, FoundNotType, FoundType };
+} // namespace
----------------
rnk wrote:
> "FoundNonType" would be closer to the standardese terminology of "non-type template parameter"
This should either be an enum class or should use a prefix for its names; these identifiers seem too general to drop into global scope here.

================
Comment at: lib/Sema/SemaDecl.cpp:139
@@ +138,3 @@
+/// type decl, \a FoundType if only type decls are found.
+static BasesLookupResult lookupInBases(Sema &S, const IdentifierInfo &II,
+                                       SourceLocation NameLoc,
----------------
The name of this function should mention something about dependent bases, since that's what's interesting about it and how it differs from normal name lookup.

================
Comment at: lib/Sema/SemaDecl.cpp:153
@@ +152,3 @@
+                        /*LookupKind=*/Sema::LookupAnyName);
+        if (!S.LookupQualifiedName(LR, DC) &&
+            LR.getResultKind() == LookupResult::NotFound)
----------------
You're going to lengths to ensure we look up into dependent bases of dependent bases, but you don't support finding names in dependent bases of non-dependent bases of dependent bases: we'll use normal name lookup here in that case. Maybe remove this whole case and do the same thing you do below: look up into the RecordDecl you get here and then recurse to its base classes.

================
Comment at: lib/Sema/SemaDecl.cpp:173-187
@@ +172,17 @@
+        continue;
+      switch (lookupInBases(S, II, NameLoc, BasePrimaryTemplate)) {
+      case FoundNotType:
+        return FoundNotType;
+      case FoundType:
+        FoundTypeDecl = FoundType;
+        break;
+      case NotFound:
+        break;
+      }
+      for (NamedDecl *ND : BasePrimaryTemplate->lookup(&II)) {
+        if (!isa<TypeDecl>(ND))
+          return FoundNotType;
+        FoundTypeDecl = FoundType;
+      }
+    }
+  }
----------------
Switch these two over: first look up in the base class itself, and then look up in its base classes if you find nothing. Otherwise you'll get the wrong result if the name is a type in one place and a non-type in the other.

================
Comment at: lib/Sema/SemaDecl.cpp:196-202
@@ -133,13 +195,9 @@
                                                       SourceLocation NameLoc) {
   // Find the first parent class template context, if any.
-  // FIXME: Perform the lookup in all enclosing class templates.
   const CXXRecordDecl *RD = nullptr;
   for (DeclContext *DC = S.CurContext; DC; DC = DC->getParent()) {
     RD = dyn_cast<CXXRecordDecl>(DC);
     if (RD && RD->getDescribedClassTemplate())
       break;
   }
   // Look for type decls in dependent base classes that have known primary
----------------
If there are multiple such base classes, we should ideally search outer ones if we find nothing in the innermost one, to match normal unqualified lookup.

http://reviews.llvm.org/D7173

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






More information about the cfe-commits mailing list