From fb6f52671ad53d3ab035a9b0f145a0b15f60efde Mon Sep 17 00:00:00 2001 From: John McCall Date: Thu, 18 Mar 2010 08:19:33 +0000 Subject: [PATCH] from code inspection, we were treating placement news with one argument as non-placement news when selecting the corresponding operator delete; this is fixed. Access and ambiguity control for calls to operator new and delete. Also AFAICT llvm-svn: 98818 --- clang/lib/Sema/Sema.h | 5 +++++ clang/lib/Sema/SemaAccess.cpp | 18 ++++++++++++++++++ clang/lib/Sema/SemaExprCXX.cpp | 23 ++++++++++++++++------- clang/test/CXX/class.access/p4.cpp | 17 +++++++++++++++++ 4 files changed, 56 insertions(+), 7 deletions(-) diff --git a/clang/lib/Sema/Sema.h b/clang/lib/Sema/Sema.h index 9251ab77e8e2..6c655e55b460 100644 --- a/clang/lib/Sema/Sema.h +++ b/clang/lib/Sema/Sema.h @@ -2628,6 +2628,11 @@ public: AccessResult CheckUnresolvedLookupAccess(UnresolvedLookupExpr *E, NamedDecl *D, AccessSpecifier Access); + AccessResult CheckAllocationAccess(SourceLocation OperatorLoc, + SourceRange PlacementRange, + CXXRecordDecl *NamingClass, + NamedDecl *Allocator, + AccessSpecifier Access); AccessResult CheckConstructorAccess(SourceLocation Loc, CXXConstructorDecl *D, AccessSpecifier Access); diff --git a/clang/lib/Sema/SemaAccess.cpp b/clang/lib/Sema/SemaAccess.cpp index 6c01fee74c32..3737c5060278 100644 --- a/clang/lib/Sema/SemaAccess.cpp +++ b/clang/lib/Sema/SemaAccess.cpp @@ -591,6 +591,24 @@ Sema::AccessResult Sema::CheckDirectMemberAccess(SourceLocation UseLoc, } +/// Checks access to an overloaded operator new or delete. +Sema::AccessResult Sema::CheckAllocationAccess(SourceLocation OpLoc, + SourceRange PlacementRange, + CXXRecordDecl *NamingClass, + NamedDecl *Fn, + AccessSpecifier Access) { + if (!getLangOptions().AccessControl || + !NamingClass || + Access == AS_public) + return AR_accessible; + + AccessedEntity Entity(AccessedEntity::Member, NamingClass, Access, Fn); + Entity.setDiag(diag::err_access) + << PlacementRange; + + return CheckAccess(*this, OpLoc, Entity); +} + /// Checks access to an overloaded member operator, including /// conversion operators. Sema::AccessResult Sema::CheckMemberOperatorAccess(SourceLocation OpLoc, diff --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp index e1e5efa7d3a9..2a55894f22a4 100644 --- a/clang/lib/Sema/SemaExprCXX.cpp +++ b/clang/lib/Sema/SemaExprCXX.cpp @@ -912,6 +912,8 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, = cast(AllocType->getAs()->getDecl()); LookupQualifiedName(FoundDelete, RD); } + if (FoundDelete.isAmbiguous()) + return true; // FIXME: clean up expressions? if (FoundDelete.empty()) { DeclareGlobalNewDelete(); @@ -919,8 +921,8 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, } FoundDelete.suppressDiagnostics(); - llvm::SmallVector Matches; - if (NumPlaceArgs > 1) { + UnresolvedSet<4> Matches; + if (NumPlaceArgs > 0) { // C++ [expr.new]p20: // A declaration of a placement deallocation function matches the // declaration of a placement allocation function if it has the @@ -962,7 +964,7 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, Fn = cast((*D)->getUnderlyingDecl()); if (Context.hasSameType(Fn->getType(), ExpectedFunctionType)) - Matches.push_back(Fn); + Matches.addDecl(Fn, D.getAccess()); } } else { // C++ [expr.new]p20: @@ -973,7 +975,7 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, D != DEnd; ++D) { if (FunctionDecl *Fn = dyn_cast((*D)->getUnderlyingDecl())) if (isNonPlacementDeallocationFunction(Fn)) - Matches.push_back(*D); + Matches.addDecl(D.getDecl(), D.getAccess()); } } @@ -982,7 +984,6 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, // function, that function will be called; otherwise, no // deallocation function will be called. if (Matches.size() == 1) { - // FIXME: Drops access, using-declaration info! OperatorDelete = cast(Matches[0]->getUnderlyingDecl()); // C++0x [expr.new]p20: @@ -998,6 +999,9 @@ bool Sema::FindAllocationFunctions(SourceLocation StartLoc, SourceRange Range, PlaceArgs[NumPlaceArgs - 1]->getLocEnd()); Diag(OperatorDelete->getLocation(), diag::note_previous_decl) << DeleteName; + } else { + CheckAllocationAccess(StartLoc, Range, FoundDelete.getNamingClass(), + Matches[0].getDecl(), Matches[0].getAccess()); } } @@ -1019,7 +1023,10 @@ bool Sema::FindAllocationOverload(SourceLocation StartLoc, SourceRange Range, << Name << Range; } - // FIXME: handle ambiguity + if (R.isAmbiguous()) + return true; + + R.suppressDiagnostics(); OverloadCandidateSet Candidates(StartLoc); for (LookupResult::iterator Alloc = R.begin(), AllocEnd = R.end(); @@ -1050,7 +1057,7 @@ bool Sema::FindAllocationOverload(SourceLocation StartLoc, SourceRange Range, // The first argument is size_t, and the first parameter must be size_t, // too. This is checked on declaration and can be assumed. (It can't be // asserted on, though, since invalid decls are left in there.) - // Whatch out for variadic allocator function. + // Watch out for variadic allocator function. unsigned NumArgsInFnDecl = FnDecl->getNumParams(); for (unsigned i = 0; (i < NumArgs && i < NumArgsInFnDecl); ++i) { if (PerformCopyInitialization(Args[i], @@ -1059,6 +1066,8 @@ bool Sema::FindAllocationOverload(SourceLocation StartLoc, SourceRange Range, return true; } Operator = FnDecl; + CheckAllocationAccess(StartLoc, Range, R.getNamingClass(), + FnDecl, Best->getAccess()); return false; } diff --git a/clang/test/CXX/class.access/p4.cpp b/clang/test/CXX/class.access/p4.cpp index d6244aaea04f..d101dcb12c0f 100644 --- a/clang/test/CXX/class.access/p4.cpp +++ b/clang/test/CXX/class.access/p4.cpp @@ -233,3 +233,20 @@ namespace test7 { } }; } + +// Ignored operator new and delete overloads are not +namespace test8 { + typedef __typeof__(sizeof(int)) size_t; + + class A { + void *operator new(size_t s); + void operator delete(void *p); + public: + void *operator new(size_t s, int n); + void operator delete(void *p, int n); + }; + + void test() { + new (2) A(); + } +}