r179029 - <rdar://problem/12806802> Propagate access specifiers for conversion functions to the conversion function set eagerly.

[Douglas Gregor]

Author: dgregor
Date: Mon Apr  8 12:12:58 2013
New Revision: 179029

URL: http://llvm.org/viewvc/llvm-project?rev=179029&view=rev
Log:
<rdar://problem/12806802> Propagate access specifiers for conversion functions to the conversion function set eagerly.

This slightly propagates an existing hack that delays when we provide
access specifiers for the visible conversion functions of a class by
copying the available access specifier early. The only client this
affects is LLDB, which tends to discover and add conversion functions
after the class is technically "complete". As such, the only
observable difference is in LLDB, so the testing will go there.


Modified:
    cfe/trunk/include/clang/AST/ASTUnresolvedSet.h
    cfe/trunk/include/clang/AST/DeclBase.h
    cfe/trunk/lib/AST/DeclCXX.cpp

Modified: cfe/trunk/include/clang/AST/ASTUnresolvedSet.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTUnresolvedSet.h?rev=179029&r1=179028&r2=179029&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/ASTUnresolvedSet.h (original)
+++ cfe/trunk/include/clang/AST/ASTUnresolvedSet.h Mon Apr  8 12:12:58 2013
@@ -41,10 +41,6 @@ public:
   const_iterator begin() const { return const_iterator(Decls.begin()); }
   const_iterator end() const { return const_iterator(Decls.end()); }
 
-  void addDecl(ASTContext &C, NamedDecl *D) {
-    addDecl(C, D, AS_none);
-  }
-
   void addDecl(ASTContext &C, NamedDecl *D, AccessSpecifier AS) {
     Decls.push_back(DeclAccessPair::make(D, AS), C);
   }
@@ -52,10 +48,13 @@ public:
   /// Replaces the given declaration with the new one, once.
   ///
   /// \return true if the set changed
-  bool replace(const NamedDecl* Old, NamedDecl *New) {
-    for (DeclsTy::iterator I = Decls.begin(), E = Decls.end(); I != E; ++I)
-      if (I->getDecl() == Old)
-        return (I->setDecl(New), true);
+  bool replace(const NamedDecl* Old, NamedDecl *New, AccessSpecifier AS) {
+    for (DeclsTy::iterator I = Decls.begin(), E = Decls.end(); I != E; ++I) {
+      if (I->getDecl() == Old) {
+        I->set(New, AS);
+        return true;
+      }
+    }
     return false;
   }
 

Modified: cfe/trunk/include/clang/AST/DeclBase.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclBase.h?rev=179029&r1=179028&r2=179029&view=diff
==============================================================================
--- cfe/trunk/include/clang/AST/DeclBase.h (original)
+++ cfe/trunk/include/clang/AST/DeclBase.h Mon Apr  8 12:12:58 2013
@@ -402,6 +402,12 @@ public:
     return AccessSpecifier(Access);
   }
 
+  /// \brief Retrieve the access specifier for this declaration, even though
+  /// it may not yet have been properly set.
+  AccessSpecifier getAccessUnsafe() const {
+    return AccessSpecifier(Access);
+  }
+
   bool hasAttrs() const { return HasAttrs; }
   void setAttrs(const AttrVec& Attrs) {
     return setAttrsImpl(Attrs, getASTContext());

Modified: cfe/trunk/lib/AST/DeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclCXX.cpp?rev=179029&r1=179028&r2=179029&view=diff
==============================================================================
--- cfe/trunk/lib/AST/DeclCXX.cpp (original)
+++ cfe/trunk/lib/AST/DeclCXX.cpp Mon Apr  8 12:12:58 2013
@@ -542,22 +542,28 @@ void CXXRecordDecl::addedMember(Decl *D)
 
     // Keep the list of conversion functions up-to-date.
     if (CXXConversionDecl *Conversion = dyn_cast<CXXConversionDecl>(D)) {
-      // FIXME: We intentionally don't use the decl's access here because it
-      // hasn't been set yet.  That's really just a misdesign in Sema.
+      // FIXME: We use the 'unsafe' accessor for the access specifier here,
+      // because Sema may not have set it yet. That's really just a misdesign
+      // in Sema. However, LLDB *will* have set the access specifier correctly,
+      // and adds declarations after the class is technically completed,
+      // so completeDefinition()'s overriding of the access specifiers doesn't
+      // work.
+      AccessSpecifier AS = Conversion->getAccessUnsafe();
+
       if (Conversion->getPrimaryTemplate()) {
         // We don't record specializations.
       } else if (FunTmpl) {
         if (FunTmpl->getPreviousDecl())
           data().Conversions.replace(FunTmpl->getPreviousDecl(),
-                                     FunTmpl);
+                                     FunTmpl, AS);
         else
-          data().Conversions.addDecl(getASTContext(), FunTmpl);
+          data().Conversions.addDecl(getASTContext(), FunTmpl, AS);
       } else {
         if (Conversion->getPreviousDecl())
           data().Conversions.replace(Conversion->getPreviousDecl(),
-                                     Conversion);
+                                     Conversion, AS);
         else
-          data().Conversions.addDecl(getASTContext(), Conversion);
+          data().Conversions.addDecl(getASTContext(), Conversion, AS);
       }
     }