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

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 12 14:04:34 PST 2015


Author: rsmith
Date: Thu Nov 12 16:04:34 2015
New Revision: 252959

URL: http://llvm.org/viewvc/llvm-project?rev=252959&view=rev
Log:
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).

Modified:
    cfe/trunk/lib/Sema/SemaLookup.cpp
    cfe/trunk/test/CXX/drs/dr4xx.cpp
    cfe/trunk/www/cxx_dr_status.html

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=252959&r1=252958&r2=252959&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Thu Nov 12 16:04:34 2015
@@ -368,13 +368,17 @@ static bool isPreferredLookupResult(Sema
   auto *DUnderlying = D->getUnderlyingDecl();
   auto *EUnderlying = Existing->getUnderlyingDecl();
 
-  // If they have different underlying declarations, pick one arbitrarily
-  // (this happens when two type declarations denote the same type).
-  // FIXME: Should we prefer a struct declaration over a typedef or vice versa?
-  //        If a name could be a typedef-name or a class-name, which is it?
+  // If they have different underlying declarations, prefer a typedef over the
+  // original type (this happens when two type declarations denote the same
+  // type), per a generous reading of C++ [dcl.typedef]p3 and p4. The typedef
+  // 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
   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,15 +510,12 @@ 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));
-        if (!UniqueResult.second) {
-          // The type is not unique.
-          ExistingI = UniqueResult.first->second;
-        }
+      QualType T = getSema().Context.getTypeDeclType(TD);
+      auto UniqueResult = UniqueTypes.insert(
+          std::make_pair(getSema().Context.getCanonicalType(T), I));
+      if (!UniqueResult.second) {
+        // The type is not unique.
+        ExistingI = UniqueResult.first->second;
       }
     }
 
@@ -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(
-            getContextForScopeMatching(Decls[UniqueTagIndex ? 0 : N - 1])))
+    NamedDecl *OtherDecl = 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;

Modified: cfe/trunk/test/CXX/drs/dr4xx.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/drs/dr4xx.cpp?rev=252959&r1=252958&r2=252959&view=diff
==============================================================================
--- cfe/trunk/test/CXX/drs/dr4xx.cpp (original)
+++ cfe/trunk/test/CXX/drs/dr4xx.cpp Thu Nov 12 16:04:34 2015
@@ -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}}
-      struct S s; // expected-error {{refers to a typedef}}
+      typedef struct S S;
+      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
-    // our behavior should not depend on the order of the using-directives.
+    // The standard doesn't say what to do in these cases either.
     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;
     }
   }
 }

Modified: cfe/trunk/www/cxx_dr_status.html
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/www/cxx_dr_status.html?rev=252959&r1=252958&r2=252959&view=diff
==============================================================================
--- cfe/trunk/www/cxx_dr_status.html (original)
+++ cfe/trunk/www/cxx_dr_status.html Thu Nov 12 16:04:34 2015
@@ -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>




More information about the cfe-commits mailing list