[cfe-commits] r94971 - in /cfe/trunk: include/clang/AST/UnresolvedSet.h lib/Sema/Sema.h lib/Sema/SemaAccess.cpp lib/Sema/SemaInit.cpp lib/Sema/SemaInit.h test/CXX/class.access/p6.cpp

John McCall rjmccall at apple.com
Sun Jan 31 19:16:55 PST 2010


Author: rjmccall
Date: Sun Jan 31 21:16:54 2010
New Revision: 94971

URL: http://llvm.org/viewvc/llvm-project?rev=94971&view=rev
Log:
Access checking for implicit user-defined conversions.


Modified:
    cfe/trunk/include/clang/AST/UnresolvedSet.h
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaAccess.cpp
    cfe/trunk/lib/Sema/SemaInit.cpp
    cfe/trunk/lib/Sema/SemaInit.h
    cfe/trunk/test/CXX/class.access/p6.cpp

Modified: cfe/trunk/include/clang/AST/UnresolvedSet.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/UnresolvedSet.h?rev=94971&r1=94970&r2=94971&view=diff

==============================================================================
--- cfe/trunk/include/clang/AST/UnresolvedSet.h (original)
+++ cfe/trunk/include/clang/AST/UnresolvedSet.h Sun Jan 31 21:16:54 2010
@@ -16,7 +16,6 @@
 #define LLVM_CLANG_AST_UNRESOLVEDSET_H
 
 #include <iterator>
-#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/SmallVector.h"
 #include "clang/Basic/Specifiers.h"
 
@@ -24,12 +23,58 @@
 
 class NamedDecl;
 
+/// A POD class for pairing a NamedDecl* with an access specifier.
+/// Can be put into unions.
+class DeclAccessPair {
+  NamedDecl *Ptr; // we'd use llvm::PointerUnion, but it isn't trivial
+
+  enum { Mask = 0x3 };
+
+public:
+  static DeclAccessPair make(NamedDecl *D, AccessSpecifier AS) {
+    DeclAccessPair p;
+    p.set(D, AS);
+    return p;
+  }
+
+  NamedDecl *getDecl() const {
+    return (NamedDecl*) (~Mask & (uintptr_t) Ptr);
+  }
+  AccessSpecifier getAccess() const {
+    return AccessSpecifier(Mask & (uintptr_t) Ptr);
+  }
+
+  void setDecl(NamedDecl *D) {
+    set(D, getAccess());
+  }
+  void setAccess(AccessSpecifier AS) {
+    set(getDecl(), AS);
+  }
+  void set(NamedDecl *D, AccessSpecifier AS) {
+    Ptr = reinterpret_cast<NamedDecl*>(uintptr_t(AS) |
+                                       reinterpret_cast<uintptr_t>(D));
+  }
+
+  operator NamedDecl*() const { return getDecl(); }
+  NamedDecl *operator->() const { return getDecl(); }
+};
+}
+
+// Take a moment to tell SmallVector that this is POD.
+namespace llvm {
+template<typename> struct isPodLike;
+template<> struct isPodLike<clang::DeclAccessPair> {
+   static const bool value = true;
+};
+}
+
+namespace clang {
+
 /// The iterator over UnresolvedSets.  Serves as both the const and
 /// non-const iterator.
 class UnresolvedSetIterator {
-
-  typedef llvm::PointerIntPair<NamedDecl*, 2> DeclEntry;
-  typedef llvm::SmallVectorImpl<DeclEntry> DeclsTy;
+private:
+  typedef llvm::SmallVectorImpl<DeclAccessPair> DeclsTy;
   typedef DeclsTy::iterator IteratorTy;
 
   IteratorTy ir;
@@ -47,8 +92,8 @@
   typedef NamedDecl *reference;
   typedef std::iterator_traits<IteratorTy>::iterator_category iterator_category;
 
-  NamedDecl *getDecl() const { return ir->getPointer(); }
-  AccessSpecifier getAccess() const { return AccessSpecifier(ir->getInt()); }
+  NamedDecl *getDecl() const { return ir->getDecl(); }
+  AccessSpecifier getAccess() const { return ir->getAccess(); }
 
   NamedDecl *operator*() const { return getDecl(); }
   
@@ -87,7 +132,6 @@
 /// in a lot of places, but isn't really worth breaking into its own
 /// header right now.
 class UnresolvedSetImpl {
-  typedef UnresolvedSetIterator::DeclEntry DeclEntry;
   typedef UnresolvedSetIterator::DeclsTy DeclsTy;
 
   // Don't allow direct construction, and only permit subclassing by
@@ -114,7 +158,7 @@
   }
 
   void addDecl(NamedDecl *D, AccessSpecifier AS) {
-    decls().push_back(DeclEntry(D, AS));
+    decls().push_back(DeclAccessPair::make(D, AS));
   }
 
   /// Replaces the given declaration with the new one, once.
@@ -122,19 +166,19 @@
   /// \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->getPointer() == Old)
-        return (I->setPointer(New), true);
+      if (I->getDecl() == Old)
+        return (I->setDecl(New), true);
     return false;
   }
 
   /// Replaces the declaration at the given iterator with the new one,
   /// preserving the original access bits.
   void replace(iterator I, NamedDecl *New) {
-    I.ir->setPointer(New);
+    I.ir->setDecl(New);
   }
 
   void replace(iterator I, NamedDecl *New, AccessSpecifier AS) {
-    *I.ir = DeclEntry(New, AS);
+    I.ir->set(New, AS);
   }
 
   void erase(unsigned I) {
@@ -148,7 +192,7 @@
   }
 
   void setAccess(iterator I, AccessSpecifier AS) {
-    I.ir->setInt(AS);
+    I.ir->setAccess(AS);
   }
 
   void clear() { decls().clear(); }
@@ -161,41 +205,8 @@
     decls().append(I.ir, E.ir);
   }
 
-  /// A proxy reference for implementing operator[].
-  class Proxy {
-    DeclEntry &Ref;
-
-    friend class UnresolvedSetImpl;
-    Proxy(DeclEntry &Ref) : Ref(Ref) {}
-
-  public:
-    NamedDecl *getDecl() const { return Ref.getPointer(); }
-    void setDecl(NamedDecl *D) { Ref.setPointer(D); }
-
-    AccessSpecifier getAccess() const { return AccessSpecifier(Ref.getInt()); }
-    void setAccess(AccessSpecifier AS) const { Ref.setInt(AS); }
-
-    NamedDecl* operator->() const { return getDecl(); }
-    operator NamedDecl*() const { return getDecl(); }
-    Proxy &operator=(const Proxy &D) { Ref = D.Ref; return *this; }
-  };
-  Proxy operator[](unsigned I) { return Proxy(decls()[I]); }
-
-  /// A proxy reference for implementing operator[] const.
-  class ConstProxy {
-    const DeclEntry &Ref;
-
-    friend class UnresolvedSetImpl;
-    ConstProxy(const DeclEntry &Ref) : Ref(Ref) {}
-
-  public:
-    NamedDecl *getDecl() const { return Ref.getPointer(); }
-    AccessSpecifier getAccess() const { return AccessSpecifier(Ref.getInt()); }
-
-    NamedDecl *operator->() const { return getDecl(); }
-    operator NamedDecl*() const { return getDecl(); }
-  };
-  ConstProxy operator[](unsigned I) const { return ConstProxy(decls()[I]); }
+  DeclAccessPair &operator[](unsigned I) { return decls()[I]; }
+  const DeclAccessPair &operator[](unsigned I) const { return decls()[I]; }
 
 private:
   // These work because the only permitted subclass is UnresolvedSetImpl
@@ -211,7 +222,7 @@
 /// A set of unresolved declarations 
 template <unsigned InlineCapacity> class UnresolvedSet :
     public UnresolvedSetImpl {
-  llvm::SmallVector<UnresolvedSetImpl::DeclEntry, InlineCapacity> Decls;
+  llvm::SmallVector<DeclAccessPair, InlineCapacity> Decls;
 };
 
   

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=94971&r1=94970&r2=94971&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Sun Jan 31 21:16:54 2010
@@ -2415,6 +2415,8 @@
   bool CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E,
                                    NamedDecl *D,
                                    AccessSpecifier Access);
+  bool CheckConstructorAccess(SourceLocation Loc, CXXConstructorDecl *D,
+                              AccessSpecifier Access);
   bool CheckMemberOperatorAccess(SourceLocation Loc, Expr *ObjectExpr,
                                  NamedDecl *D, AccessSpecifier Access);
   bool CheckAccess(const LookupResult &R, NamedDecl *D, AccessSpecifier Access);

Modified: cfe/trunk/lib/Sema/SemaAccess.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaAccess.cpp?rev=94971&r1=94970&r2=94971&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaAccess.cpp (original)
+++ cfe/trunk/lib/Sema/SemaAccess.cpp Sun Jan 31 21:16:54 2010
@@ -306,7 +306,24 @@
   return false;
 }
 
-/// Checks access to an overloaded member operator.
+/// Checks access to a constructor.
+bool Sema::CheckConstructorAccess(SourceLocation UseLoc,
+                                  CXXConstructorDecl *Constructor,
+                                  AccessSpecifier Access) {
+  if (!getLangOptions().AccessControl)
+    return false;
+
+  CXXRecordDecl *NamingClass = cast<CXXRecordDecl>(Constructor->getParent());
+
+  LookupResult R(*this, Constructor->getDeclName(), UseLoc, LookupOrdinaryName);
+  R.suppressDiagnostics();
+
+  R.setNamingClass(NamingClass);
+  return CheckAccess(R, Constructor, Access);
+}
+
+/// Checks access to an overloaded member operator, including
+/// conversion operators.
 bool Sema::CheckMemberOperatorAccess(SourceLocation OpLoc,
                                      Expr *ObjectExpr,
                                      NamedDecl *MemberOperator,

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=94971&r1=94970&r2=94971&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Sun Jan 31 21:16:54 2010
@@ -1971,7 +1971,8 @@
   Step S;
   S.Kind = SK_ResolveAddressOfOverloadedFunction;
   S.Type = Function->getType();
-  S.Function = Function;
+  // Access is currently ignored for these.
+  S.Function = DeclAccessPair::make(Function, AccessSpecifier(0));
   Steps.push_back(S);
 }
 
@@ -1992,11 +1993,12 @@
 }
 
 void InitializationSequence::AddUserConversionStep(FunctionDecl *Function,
+                                                   AccessSpecifier Access,
                                                    QualType T) {
   Step S;
   S.Kind = SK_UserConversion;
   S.Type = T;
-  S.Function = Function;
+  S.Function = DeclAccessPair::make(Function, Access);
   Steps.push_back(S);
 }
 
@@ -2029,11 +2031,12 @@
 void 
 InitializationSequence::AddConstructorInitializationStep(
                                               CXXConstructorDecl *Constructor,
+                                                       AccessSpecifier Access,
                                                          QualType T) {
   Step S;
   S.Kind = SK_ConstructorInitialization;
   S.Type = T;
-  S.Function = Constructor;
+  S.Function = DeclAccessPair::make(Constructor, Access);
   Steps.push_back(S);
 }
 
@@ -2246,7 +2249,8 @@
     T2 = cv1T1;
 
   // Add the user-defined conversion step.
-  Sequence.AddUserConversionStep(Function, T2.getNonReferenceType());
+  Sequence.AddUserConversionStep(Function, Best->getAccess(),
+                                 T2.getNonReferenceType());
 
   // Determine whether we need to perform derived-to-base or 
   // cv-qualification adjustments.
@@ -2577,10 +2581,11 @@
   // Add the constructor initialization step. Any cv-qualification conversion is
   // subsumed by the initialization.
   if (Kind.getKind() == InitializationKind::IK_Copy) {
-    Sequence.AddUserConversionStep(Best->Function, DestType);
+    Sequence.AddUserConversionStep(Best->Function, Best->getAccess(), DestType);
   } else {
     Sequence.AddConstructorInitializationStep(
                                       cast<CXXConstructorDecl>(Best->Function), 
+                                      Best->getAccess(),
                                       DestType);
   }
 }
@@ -2778,13 +2783,13 @@
   if (isa<CXXConstructorDecl>(Function)) {
     // Add the user-defined conversion step. Any cv-qualification conversion is
     // subsumed by the initialization.
-    Sequence.AddUserConversionStep(Function, DestType);
+    Sequence.AddUserConversionStep(Function, Best->getAccess(), DestType);
     return;
   }
 
   // Add the user-defined conversion step that calls the conversion function.
   QualType ConvType = Function->getResultType().getNonReferenceType();
-  Sequence.AddUserConversionStep(Function, ConvType);
+  Sequence.AddUserConversionStep(Function, Best->getAccess(), ConvType);
 
   // If the conversion following the call to the conversion function is 
   // interesting, add it as a separate step.
@@ -3250,7 +3255,9 @@
     case SK_ResolveAddressOfOverloadedFunction:
       // Overload resolution determined which function invoke; update the 
       // initializer to reflect that choice.
-      CurInit = S.FixOverloadedFunctionReference(move(CurInit), Step->Function);
+      // Access control was done in overload resolution.
+      CurInit = S.FixOverloadedFunctionReference(move(CurInit),
+                              cast<FunctionDecl>(Step->Function.getDecl()));
       break;
         
     case SK_CastDerivedToBaseRValue:
@@ -3329,13 +3336,14 @@
       // or a conversion function.
       CastExpr::CastKind CastKind = CastExpr::CK_Unknown;
       bool IsCopy = false;
-      if (CXXConstructorDecl *Constructor
-                              = dyn_cast<CXXConstructorDecl>(Step->Function)) {
+      FunctionDecl *Fn = cast<FunctionDecl>(Step->Function.getDecl());
+      AccessSpecifier FnAccess = Step->Function.getAccess();
+      if (CXXConstructorDecl *Constructor = dyn_cast<CXXConstructorDecl>(Fn)) {
         // Build a call to the selected constructor.
         ASTOwningVector<&ActionBase::DeleteExpr> ConstructorArgs(S);
         SourceLocation Loc = CurInitExpr->getLocStart();
         CurInit.release(); // Ownership transferred into MultiExprArg, below.
-        
+
         // Determine the arguments required to actually perform the constructor
         // call.
         if (S.CompleteConstructorCall(Constructor,
@@ -3350,6 +3358,8 @@
                                           move_arg(ConstructorArgs));
         if (CurInit.isInvalid())
           return S.ExprError();
+
+        S.CheckConstructorAccess(Kind.getLocation(), Constructor, FnAccess);
         
         CastKind = CastExpr::CK_ConstructorConversion;
         QualType Class = S.Context.getTypeDeclType(Constructor->getParent());
@@ -3358,8 +3368,11 @@
           IsCopy = true;
       } else {
         // Build a call to the conversion function.
-        CXXConversionDecl *Conversion = cast<CXXConversionDecl>(Step->Function);
+        CXXConversionDecl *Conversion = cast<CXXConversionDecl>(Fn);
 
+        S.CheckMemberOperatorAccess(Kind.getLocation(), CurInitExpr,
+                                    Conversion, FnAccess);
+        
         // FIXME: Should we move this initialization into a separate 
         // derived-to-base conversion? I believe the answer is "no", because
         // we don't want to turn off access control here for c-style casts.
@@ -3424,8 +3437,8 @@
 
     case SK_ConstructorInitialization: {
       CXXConstructorDecl *Constructor
-        = cast<CXXConstructorDecl>(Step->Function);
-      
+        = cast<CXXConstructorDecl>(Step->Function.getDecl());
+
       // Build a call to the selected constructor.
       ASTOwningVector<&ActionBase::DeleteExpr> ConstructorArgs(S);
       SourceLocation Loc = Kind.getLocation();
@@ -3444,6 +3457,9 @@
                                Entity.getKind() == InitializedEntity::EK_Base);
       if (CurInit.isInvalid())
         return S.ExprError();
+
+      // Only check access if all of that succeeded.
+      S.CheckConstructorAccess(Loc, Constructor, Step->Function.getAccess());
       
       bool Elidable 
         = cast<CXXConstructExpr>((Expr *)CurInit.get())->isElidable();

Modified: cfe/trunk/lib/Sema/SemaInit.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.h?rev=94971&r1=94970&r2=94971&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaInit.h (original)
+++ cfe/trunk/lib/Sema/SemaInit.h Sun Jan 31 21:16:54 2010
@@ -15,6 +15,7 @@
 
 #include "SemaOverload.h"
 #include "clang/AST/Type.h"
+#include "clang/AST/UnresolvedSet.h"
 #include "clang/Parse/Action.h"
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/ADT/PointerIntPair.h"
@@ -449,7 +450,11 @@
       /// \brief When Kind == SK_ResolvedOverloadedFunction or Kind ==
       /// SK_UserConversion, the function that the expression should be 
       /// resolved to or the conversion function to call, respectively.
-      FunctionDecl *Function;
+      ///
+      /// Always a FunctionDecl.
+      /// For conversion decls, the naming class is the source type.
+      /// For construct decls, the naming class is the target type.
+      DeclAccessPair Function;
       
       /// \brief When Kind = SK_ConversionSequence, the implicit conversion
       /// sequence 
@@ -616,7 +621,9 @@
   
   /// \brief Add a new step invoking a conversion function, which is either
   /// a constructor or a conversion function.
-  void AddUserConversionStep(FunctionDecl *Function, QualType T);
+  void AddUserConversionStep(FunctionDecl *Function,
+                             AccessSpecifier Access,
+                             QualType T);
   
   /// \brief Add a new step that performs a qualification conversion to the
   /// given type.
@@ -631,6 +638,7 @@
 
   /// \brief Add a constructor-initialization step.
   void AddConstructorInitializationStep(CXXConstructorDecl *Constructor,
+                                        AccessSpecifier Access,
                                         QualType T);
 
   /// \brief Add a zero-initialization step.

Modified: cfe/trunk/test/CXX/class.access/p6.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class.access/p6.cpp?rev=94971&r1=94970&r2=94971&view=diff

==============================================================================
--- cfe/trunk/test/CXX/class.access/p6.cpp (original)
+++ cfe/trunk/test/CXX/class.access/p6.cpp Sun Jan 31 21:16:54 2010
@@ -15,6 +15,8 @@
 //   to create and destroy a static data member is performed as if
 //   these calls appeared in the scope of the member's class.
 
+struct Public {}; struct Protected {}; struct Private {};
+
 namespace test0 {
   class A {
     typedef int type; // expected-note {{declared private here}}
@@ -24,3 +26,29 @@
   A::type foo() { } // expected-error {{access to private member}}
   A::type A::foo() { }
 }
+
+// conversion decls
+namespace test1 {
+  class A {
+  public:
+    A();
+    operator Public ();
+    A(Public);
+  protected:
+    operator Protected (); // expected-note {{declared protected here}}
+    A(Protected); // expected-note {{declared protected here}}
+  private:
+    operator Private (); // expected-note {{declared private here}}
+    A(Private); // expected-note {{declared private here}}
+  };
+
+  void test() {
+    A a;
+    Public pub = a;
+    Protected prot = a; // expected-error {{access to protected member}}
+    Private priv = a; // expected-error {{access to private member}}
+    A apub = pub;
+    A aprot = prot; // expected-error {{access to protected member}}
+    A apriv = priv; // expected-error {{access to private member}}
+  }
+}





More information about the cfe-commits mailing list