DR407: Rationalize how we handle tags being hidden by typedefs. Even with

DR407, the C++ standard doesn't really say how this should work. Here's what we
do (which is consistent with DR407 as far as I can tell):

 * When performing name lookup for an elaborated-type-specifier, a tag
   declaration hides a typedef declaration that names the same type.
 * When performing any other kind of lookup, a typedef declaration hides
   a tag declaration that names the same type.

In any other case where lookup finds both a typedef and a tag (that is, when
they name different types), the lookup will be ambiguous. If lookup finds a
tag and a typedef that name the same type, and finds anything else, the lookup
will always be ambiguous (even if the other entity would hide the tag, it does
not also hide the typedef).


git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@252959 91177308-0d34-0410-b5e6-96231b3b80d8

lib/Sema/SemaLookup.cpp

@@ -368,13 +368,17 @@ static bool isPreferredLookupResult(Sema &S, Sema::LookupNameKind Kind,
   auto *DUnderlying = D->getUnderlyingDecl();
   auto *EUnderlying = Existing->getUnderlyingDecl();
 
-  // If they have different underlying declarations, pick one arbitrarily
+  // If they have different underlying declarations, prefer a typedef over the
-  // (this happens when two type declarations denote the same type).
+  // original type (this happens when two type declarations denote the same
-  // FIXME: Should we prefer a struct declaration over a typedef or vice versa?
+  // type), per a generous reading of C++ [dcl.typedef]p3 and p4. The typedef
-  //        If a name could be a typedef-name or a class-name, which is it?
+  // might carry additional semantic information, such as an alignment override.
+  // However, per C++ [dcl.typedef]p5, when looking up a tag name, prefer a tag
+  // declaration over a typedef.
   if (DUnderlying->getCanonicalDecl() != EUnderlying->getCanonicalDecl()) {
     assert(isa<TypeDecl>(DUnderlying) && isa<TypeDecl>(EUnderlying));
-    return false;
+    bool HaveTag = isa<TagDecl>(EUnderlying);
+    bool WantTag = Kind == Sema::LookupTagName;
+    return HaveTag != WantTag;
   }
 
   // Pick the function with more default arguments.
@@ -434,6 +438,23 @@ static bool isPreferredLookupResult(Sema &S, Sema::LookupNameKind Kind,
   return !S.isVisible(Existing);
 }
 
+/// Determine whether \p D can hide a tag declaration.
+static bool canHideTag(NamedDecl *D) {
+  // C++ [basic.scope.declarative]p4:
+  //   Given a set of declarations in a single declarative region [...]
+  //   exactly one declaration shall declare a class name or enumeration name
+  //   that is not a typedef name and the other declarations shall all refer to
+  //   the same variable or enumerator, or all refer to functions and function
+  //   templates; in this case the class name or enumeration name is hidden.
+  // C++ [basic.scope.hiding]p2:
+  //   A class name or enumeration name can be hidden by the name of a
+  //   variable, data member, function, or enumerator declared in the same
+  //   scope.
+  D = D->getUnderlyingDecl();
+  return isa<VarDecl>(D) || isa<EnumConstantDecl>(D) || isa<FunctionDecl>(D) ||
+         isa<FunctionTemplateDecl>(D) || isa<FieldDecl>(D);
+}
+
 /// Resolves the result kind of this lookup.
 void LookupResult::resolveKind() {
   unsigned N = Decls.size();
@@ -489,8 +510,6 @@ void LookupResult::resolveKind() {
     // no ambiguity if they all refer to the same type, so unique based on the
     // canonical type.
     if (TypeDecl *TD = dyn_cast<TypeDecl>(D)) {
-      // FIXME: Why are nested type declarations treated differently?
-      if (!TD->getDeclContext()->isRecord()) {
       QualType T = getSema().Context.getTypeDeclType(TD);
       auto UniqueResult = UniqueTypes.insert(
           std::make_pair(getSema().Context.getCanonicalType(T), I));
@@ -499,7 +518,6 @@ void LookupResult::resolveKind() {
         ExistingI = UniqueResult.first->second;
       }
     }
-    }
 
     // For non-type declarations, check for a prior lookup result naming this
     // canonical declaration.
@@ -564,10 +582,13 @@ void LookupResult::resolveKind() {
   //   wherever the object, function, or enumerator name is visible.
   // But it's still an error if there are distinct tag types found,
   // even if they're not visible. (ref?)
-  if (HideTags && HasTag && !Ambiguous &&
+  if (N > 1 && HideTags && HasTag && !Ambiguous &&
       (HasFunction || HasNonFunction || HasUnresolved)) {
-    if (getContextForScopeMatching(Decls[UniqueTagIndex])->Equals(
+    NamedDecl *OtherDecl = Decls[UniqueTagIndex ? 0 : N - 1];
-            getContextForScopeMatching(Decls[UniqueTagIndex ? 0 : N - 1])))
+    if (isa<TagDecl>(Decls[UniqueTagIndex]->getUnderlyingDecl()) &&
+        getContextForScopeMatching(Decls[UniqueTagIndex])->Equals(
+            getContextForScopeMatching(OtherDecl)) &&
+        canHideTag(OtherDecl))
       Decls[UniqueTagIndex] = Decls[--N];
     else
       Ambiguous = true;

test/CXX/drs/dr4xx.cpp

@@ -83,7 +83,7 @@ namespace dr406 { // dr406: yes
   } A;
 }
 
-namespace dr407 { // dr407: no
+namespace dr407 { // dr407: 3.8
   struct S;
   typedef struct S S;
   void f() {
@@ -108,22 +108,22 @@ namespace dr407 { // dr407: no
       struct S s; // expected-error {{ambiguous}}
     }
     namespace D {
-      // FIXME: This is valid.
       using A::S;
-      typedef struct S S; // expected-note {{here}}
+      typedef struct S S;
-      struct S s; // expected-error {{refers to a typedef}}
+      struct S s;
     }
     namespace E {
-      // FIXME: The standard doesn't say whether this is valid.
+      // The standard doesn't say whether this is valid. We interpret
+      // DR407 as meaning "if lookup finds both a tag and a typedef with the
+      // same type, then it's OK in an elaborated-type-specifier".
       typedef A::S S;
       using A::S;
       struct S s;
     }
     namespace F {
-      typedef A::S S; // expected-note {{here}}
+      typedef A::S S;
     }
-    // FIXME: The standard doesn't say what to do in these cases, but
+    // The standard doesn't say what to do in these cases either.
-    // our behavior should not depend on the order of the using-directives.
     namespace G {
       using namespace A;
       using namespace F;
@@ -132,7 +132,7 @@ namespace dr407 { // dr407: no
     namespace H {
       using namespace F;
       using namespace A;
-      struct S s; // expected-error {{refers to a typedef}}
+      struct S s;
     }
   }
 }

www/cxx_dr_status.html

@@ -2483,7 +2483,7 @@ of class templates</td>
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#407">407</a></td>
     <td>C++11</td>
     <td>Named class with associated typedef: two names or one?</td>
-    <td class="none" align="center">No</td>
+    <td class="svn" align="center">SVN</td>
   </tr>
   <tr id="408">
     <td><a href="http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#408">408</a></td>