From 2e0c8f79d97b974a680087d12a7dc5e1dc705c20 Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Sat, 27 Dec 2014 03:58:08 +0000 Subject: [PATCH] Address review feedback on r221933. Remove ObjCMethodList::Count, instead store a "has more than one decl" bit in the low bit of the ObjCMethodDecl pointer, using a PointerIntPair. Most of this patch is replacing ".Method" with ".getMethod()". No intended behavior change. llvm-svn: 224876 --- clang/include/clang/Sema/ObjCMethodList.h | 28 +++-- clang/lib/Sema/SemaCodeComplete.cpp | 32 +++--- clang/lib/Sema/SemaDeclObjC.cpp | 118 ++++++++++++---------- clang/lib/Sema/SemaExprObjC.cpp | 7 +- clang/lib/Serialization/ASTReader.cpp | 14 +-- clang/lib/Serialization/ASTWriter.cpp | 28 ++--- 6 files changed, 125 insertions(+), 102 deletions(-) diff --git a/clang/include/clang/Sema/ObjCMethodList.h b/clang/include/clang/Sema/ObjCMethodList.h index 956e0882c5f3..6c759d922ae8 100644 --- a/clang/include/clang/Sema/ObjCMethodList.h +++ b/clang/include/clang/Sema/ObjCMethodList.h @@ -20,22 +20,36 @@ namespace clang { class ObjCMethodDecl; -/// ObjCMethodList - a linked list of methods with different signatures. +/// \brief a linked list of methods with the same selector name but different +/// signatures. struct ObjCMethodList { - ObjCMethodDecl *Method; - /// \brief count of methods with same signature. - unsigned Count; + /// \brief If there is more than one decl with this signature. + llvm::PointerIntPair MethodAndHasMoreThanOneDecl; /// \brief The next list object and 2 bits for extra info. llvm::PointerIntPair NextAndExtraBits; - ObjCMethodList() : Method(nullptr), Count(0) { } - ObjCMethodList(ObjCMethodDecl *M, unsigned count, ObjCMethodList *C) - : Method(M), Count(count), NextAndExtraBits(C, 0) { } + ObjCMethodList() { } + ObjCMethodList(ObjCMethodDecl *M) + : MethodAndHasMoreThanOneDecl(M, 0) {} ObjCMethodList *getNext() const { return NextAndExtraBits.getPointer(); } unsigned getBits() const { return NextAndExtraBits.getInt(); } void setNext(ObjCMethodList *L) { NextAndExtraBits.setPointer(L); } void setBits(unsigned B) { NextAndExtraBits.setInt(B); } + + ObjCMethodDecl *getMethod() const { + return MethodAndHasMoreThanOneDecl.getPointer(); + } + void setMethod(ObjCMethodDecl *M) { + return MethodAndHasMoreThanOneDecl.setPointer(M); + } + + bool hasMoreThanOneDecl() const { + return MethodAndHasMoreThanOneDecl.getInt(); + } + void setHasMoreThanOneDecl(bool B) { + return MethodAndHasMoreThanOneDecl.setInt(B); + } }; } diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp index 50d51348f4a1..48bdd2a7d82c 100644 --- a/clang/lib/Sema/SemaCodeComplete.cpp +++ b/clang/lib/Sema/SemaCodeComplete.cpp @@ -5404,13 +5404,13 @@ static void AddClassMessageCompletions(Sema &SemaRef, Scope *S, MEnd = SemaRef.MethodPool.end(); M != MEnd; ++M) { for (ObjCMethodList *MethList = &M->second.second; - MethList && MethList->Method; + MethList && MethList->getMethod(); MethList = MethList->getNext()) { - if (!isAcceptableObjCMethod(MethList->Method, MK_Any, SelIdents)) + if (!isAcceptableObjCMethod(MethList->getMethod(), MK_Any, SelIdents)) continue; - Result R(MethList->Method, Results.getBasePriority(MethList->Method), - nullptr); + Result R(MethList->getMethod(), + Results.getBasePriority(MethList->getMethod()), nullptr); R.StartParameter = SelIdents.size(); R.AllParametersAreInformative = false; Results.MaybeAddResult(R, SemaRef.CurContext); @@ -5577,16 +5577,16 @@ void Sema::CodeCompleteObjCInstanceMessage(Scope *S, Expr *Receiver, MEnd = MethodPool.end(); M != MEnd; ++M) { for (ObjCMethodList *MethList = &M->second.first; - MethList && MethList->Method; + MethList && MethList->getMethod(); MethList = MethList->getNext()) { - if (!isAcceptableObjCMethod(MethList->Method, MK_Any, SelIdents)) + if (!isAcceptableObjCMethod(MethList->getMethod(), MK_Any, SelIdents)) continue; - if (!Selectors.insert(MethList->Method->getSelector()).second) + if (!Selectors.insert(MethList->getMethod()->getSelector()).second) continue; - Result R(MethList->Method, Results.getBasePriority(MethList->Method), - nullptr); + Result R(MethList->getMethod(), + Results.getBasePriority(MethList->getMethod()), nullptr); R.StartParameter = SelIdents.size(); R.AllParametersAreInformative = false; Results.MaybeAddResult(R, CurContext); @@ -6994,16 +6994,18 @@ void Sema::CodeCompleteObjCMethodDeclSelector(Scope *S, M != MEnd; ++M) { for (ObjCMethodList *MethList = IsInstanceMethod ? &M->second.first : &M->second.second; - MethList && MethList->Method; + MethList && MethList->getMethod(); MethList = MethList->getNext()) { - if (!isAcceptableObjCMethod(MethList->Method, MK_Any, SelIdents)) + if (!isAcceptableObjCMethod(MethList->getMethod(), MK_Any, SelIdents)) continue; if (AtParameterName) { // Suggest parameter names we've seen before. unsigned NumSelIdents = SelIdents.size(); - if (NumSelIdents && NumSelIdents <= MethList->Method->param_size()) { - ParmVarDecl *Param = MethList->Method->parameters()[NumSelIdents-1]; + if (NumSelIdents && + NumSelIdents <= MethList->getMethod()->param_size()) { + ParmVarDecl *Param = + MethList->getMethod()->parameters()[NumSelIdents - 1]; if (Param->getIdentifier()) { CodeCompletionBuilder Builder(Results.getAllocator(), Results.getCodeCompletionTUInfo()); @@ -7016,8 +7018,8 @@ void Sema::CodeCompleteObjCMethodDeclSelector(Scope *S, continue; } - Result R(MethList->Method, Results.getBasePriority(MethList->Method), - nullptr); + Result R(MethList->getMethod(), + Results.getBasePriority(MethList->getMethod()), nullptr); R.StartParameter = SelIdents.size(); R.AllParametersAreInformative = false; R.DeclaringEntity = true; diff --git a/clang/lib/Sema/SemaDeclObjC.cpp b/clang/lib/Sema/SemaDeclObjC.cpp index eccf2e1d2ae0..90835be722bc 100644 --- a/clang/lib/Sema/SemaDeclObjC.cpp +++ b/clang/lib/Sema/SemaDeclObjC.cpp @@ -2004,13 +2004,12 @@ void Sema::ImplMethodsVsClassMethods(Scope *S, ObjCImplDecl* IMPDecl, IncompleteImpl, InsMap, ClsMap, CDecl, ExplicitImplProtocols); DiagnoseUnimplementedProperties(S, IMPDecl, CDecl, - /* SynthesizeProperties */ false); + /*SynthesizeProperties=*/false); } } else llvm_unreachable("invalid ObjCContainerDecl type."); } -/// ActOnForwardClassDeclaration - Sema::DeclGroupPtrTy Sema::ActOnForwardClassDeclaration(SourceLocation AtClassLoc, IdentifierInfo **IdentList, @@ -2037,10 +2036,11 @@ Sema::ActOnForwardClassDeclaration(SourceLocation AtClassLoc, } else { // a forward class declaration matching a typedef name of a class refers // to the underlying class. Just ignore the forward class with a warning - // as this will force the intended behavior which is to lookup the typedef - // name. + // as this will force the intended behavior which is to lookup the + // typedef name. if (isa(TDD->getUnderlyingType())) { - Diag(AtClassLoc, diag::warn_forward_class_redefinition) << IdentList[i]; + Diag(AtClassLoc, diag::warn_forward_class_redefinition) + << IdentList[i]; Diag(PrevDecl->getLocation(), diag::note_previous_definition); continue; } @@ -2217,22 +2217,22 @@ bool Sema::MatchTwoMethodDeclarations(const ObjCMethodDecl *left, return true; } -void Sema::addMethodToGlobalList(ObjCMethodList *List, ObjCMethodDecl *Method) { +void Sema::addMethodToGlobalList(ObjCMethodList *List, + ObjCMethodDecl *Method) { // Record at the head of the list whether there were 0, 1, or >= 2 methods // inside categories. - if (ObjCCategoryDecl * - CD = dyn_cast(Method->getDeclContext())) + if (ObjCCategoryDecl *CD = + dyn_cast(Method->getDeclContext())) if (!CD->IsClassExtension() && List->getBits() < 2) - List->setBits(List->getBits()+1); + List->setBits(List->getBits() + 1); // If the list is empty, make it a singleton list. - if (List->Method == nullptr) { - List->Method = Method; + if (List->getMethod() == nullptr) { + List->setMethod(Method); List->setNext(nullptr); - List->Count = Method->isDefined() ? 0 : 1; return; } - + // We've seen a method with this name, see if we have already seen this type // signature. ObjCMethodList *Previous = List; @@ -2241,37 +2241,42 @@ void Sema::addMethodToGlobalList(ObjCMethodList *List, ObjCMethodDecl *Method) { if (getLangOpts().Modules && !getLangOpts().CurrentModule.empty()) continue; - if (!MatchTwoMethodDeclarations(Method, List->Method)) + if (!MatchTwoMethodDeclarations(Method, List->getMethod())) continue; - - ObjCMethodDecl *PrevObjCMethod = List->Method; + + ObjCMethodDecl *PrevObjCMethod = List->getMethod(); // Propagate the 'defined' bit. if (Method->isDefined()) PrevObjCMethod->setDefined(true); - else - ++List->Count; - + else if (!PrevObjCMethod->isDefined()) { + // Objective-C doesn't allow an @interface for a class after its + // @implementation. So if Method is not defined and there already is + // an entry for this type signature, Method has to be for a different + // class than PrevObjCMethod. + List->setHasMoreThanOneDecl(true); + } + // If a method is deprecated, push it in the global pool. // This is used for better diagnostics. if (Method->isDeprecated()) { if (!PrevObjCMethod->isDeprecated()) - List->Method = Method; + List->setMethod(Method); } - // If new method is unavailable, push it into global pool + // If the new method is unavailable, push it into global pool // unless previous one is deprecated. if (Method->isUnavailable()) { if (PrevObjCMethod->getAvailability() < AR_Deprecated) - List->Method = Method; + List->setMethod(Method); } - + return; } - + // We have a new signature for an existing method - add it. // This is extremely rare. Only 1% of Cocoa selectors are "overloaded". ObjCMethodList *Mem = BumpAlloc.Allocate(); - Previous->setNext(new (Mem) ObjCMethodList(Method, 0, nullptr)); + Previous->setNext(new (Mem) ObjCMethodList(Method)); } /// \brief Read the contents of the method pool for a given selector from @@ -2294,7 +2299,7 @@ void Sema::AddMethodToGlobalPool(ObjCMethodDecl *Method, bool impl, if (Pos == MethodPool.end()) Pos = MethodPool.insert(std::make_pair(Method->getSelector(), GlobalMethods())).first; - + Method->setDefined(impl); ObjCMethodList &Entry = instance ? Pos->second.first : Pos->second.second; @@ -2320,9 +2325,8 @@ static bool isAcceptableMethodMismatch(ObjCMethodDecl *chosen, return (chosen->getReturnType()->isIntegerType()); } -bool Sema::CollectMultipleMethodsInGlobalPool(Selector Sel, - SmallVectorImpl& Methods, - bool instance) { +bool Sema::CollectMultipleMethodsInGlobalPool( + Selector Sel, SmallVectorImpl &Methods, bool instance) { if (ExternalSource) ReadMethodPool(Sel); @@ -2332,19 +2336,19 @@ bool Sema::CollectMultipleMethodsInGlobalPool(Selector Sel, // Gather the non-hidden methods. ObjCMethodList &MethList = instance ? Pos->second.first : Pos->second.second; for (ObjCMethodList *M = &MethList; M; M = M->getNext()) - if (M->Method && !M->Method->isHidden()) - Methods.push_back(M->Method); - return (Methods.size() > 1); + if (M->getMethod() && !M->getMethod()->isHidden()) + Methods.push_back(M->getMethod()); + return Methods.size() > 1; } -bool Sema::AreMultipleMethodsInGlobalPool(Selector Sel, - bool instance) { +bool Sema::AreMultipleMethodsInGlobalPool(Selector Sel, bool instance) { GlobalMethodPool::iterator Pos = MethodPool.find(Sel); - // Test for no method in the pool which should not trigger any warning by caller. + // Test for no method in the pool which should not trigger any warning by + // caller. if (Pos == MethodPool.end()) return true; ObjCMethodList &MethList = instance ? Pos->second.first : Pos->second.second; - return MethList.Count > 1; + return MethList.hasMoreThanOneDecl(); } ObjCMethodDecl *Sema::LookupMethodInGlobalPool(Selector Sel, SourceRange R, @@ -2361,12 +2365,12 @@ ObjCMethodDecl *Sema::LookupMethodInGlobalPool(Selector Sel, SourceRange R, ObjCMethodList &MethList = instance ? Pos->second.first : Pos->second.second; SmallVector Methods; for (ObjCMethodList *M = &MethList; M; M = M->getNext()) { - if (M->Method && !M->Method->isHidden()) { + if (M->getMethod() && !M->getMethod()->isHidden()) { // If we're not supposed to warn about mismatches, we're done. if (!warn) - return M->Method; + return M->getMethod(); - Methods.push_back(M->Method); + Methods.push_back(M->getMethod()); } } @@ -2438,13 +2442,13 @@ ObjCMethodDecl *Sema::LookupImplementedMethodInGlobalPool(Selector Sel) { GlobalMethods &Methods = Pos->second; for (const ObjCMethodList *Method = &Methods.first; Method; Method = Method->getNext()) - if (Method->Method && Method->Method->isDefined()) - return Method->Method; + if (Method->getMethod() && Method->getMethod()->isDefined()) + return Method->getMethod(); for (const ObjCMethodList *Method = &Methods.second; Method; Method = Method->getNext()) - if (Method->Method && Method->Method->isDefined()) - return Method->Method; + if (Method->getMethod() && Method->getMethod()->isDefined()) + return Method->getMethod(); return nullptr; } @@ -2507,25 +2511,27 @@ Sema::SelectorsForTypoCorrection(Selector Sel, e = MethodPool.end(); b != e; b++) { // instance methods for (ObjCMethodList *M = &b->second.first; M; M=M->getNext()) - if (M->Method && - (M->Method->getSelector().getNumArgs() == NumArgs) && - (M->Method->getSelector() != Sel)) { + if (M->getMethod() && + (M->getMethod()->getSelector().getNumArgs() == NumArgs) && + (M->getMethod()->getSelector() != Sel)) { if (ObjectIsId) - Methods.push_back(M->Method); + Methods.push_back(M->getMethod()); else if (!ObjectIsClass && - HelperIsMethodInObjCType(*this, M->Method->getSelector(), ObjectType)) - Methods.push_back(M->Method); + HelperIsMethodInObjCType(*this, M->getMethod()->getSelector(), + ObjectType)) + Methods.push_back(M->getMethod()); } // class methods for (ObjCMethodList *M = &b->second.second; M; M=M->getNext()) - if (M->Method && - (M->Method->getSelector().getNumArgs() == NumArgs) && - (M->Method->getSelector() != Sel)) { + if (M->getMethod() && + (M->getMethod()->getSelector().getNumArgs() == NumArgs) && + (M->getMethod()->getSelector() != Sel)) { if (ObjectIsClass) - Methods.push_back(M->Method); + Methods.push_back(M->getMethod()); else if (!ObjectIsId && - HelperIsMethodInObjCType(*this, M->Method->getSelector(), ObjectType)) - Methods.push_back(M->Method); + HelperIsMethodInObjCType(*this, M->getMethod()->getSelector(), + ObjectType)) + Methods.push_back(M->getMethod()); } } @@ -2855,7 +2861,7 @@ public: } ObjCMethodList &list = method->isInstanceMethod() ? it->second.first : it->second.second; - if (!list.Method) return; + if (!list.getMethod()) return; ObjCContainerDecl *container = cast(method->getDeclContext()); diff --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp index 79f7d92e98ce..9c3b51c623d3 100644 --- a/clang/lib/Sema/SemaExprObjC.cpp +++ b/clang/lib/Sema/SemaExprObjC.cpp @@ -987,7 +987,7 @@ static bool HelperToDiagnoseMismatchedMethodsInGlobalPool(Sema &S, ObjCMethodList *M = &MethList; bool Warned = false; for (M = M->getNext(); M; M=M->getNext()) { - ObjCMethodDecl *MatchingMethodDecl = M->Method; + ObjCMethodDecl *MatchingMethodDecl = M->getMethod(); if (MatchingMethodDecl == Method || isa(MatchingMethodDecl->getDeclContext()) || MatchingMethodDecl->getSelector() != Method->getSelector()) @@ -2458,8 +2458,9 @@ ExprResult Sema::BuildInstanceMessage(Expr *Receiver, } else if (ReceiverType->isObjCClassType() || ReceiverType->isObjCQualifiedClassType()) { // Handle messages to Class. - // We allow sending a message to a qualified Class ("Class"), which - // is ok as long as one of the protocols implements the selector (if not, warn). + // We allow sending a message to a qualified Class ("Class"), which + // is ok as long as one of the protocols implements the selector (if not, + // warn). if (const ObjCObjectPointerType *QClassTy = ReceiverType->getAsObjCQualifiedClassType()) { // Search protocols for class methods. diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index eea4ba84b0c3..b3bac25a0767 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3457,7 +3457,7 @@ static void moveMethodToBackOfGlobalList(Sema &S, ObjCMethodDecl *Method) { bool Found = false; for (ObjCMethodList *List = &Start; List; List = List->getNext()) { if (!Found) { - if (List->Method == Method) { + if (List->getMethod() == Method) { Found = true; } else { // Keep searching. @@ -3466,9 +3466,9 @@ static void moveMethodToBackOfGlobalList(Sema &S, ObjCMethodDecl *Method) { } if (List->getNext()) - List->Method = List->getNext()->Method; + List->setMethod(List->getNext()->getMethod()); else - List->Method = Method; + List->setMethod(Method); } } @@ -7065,11 +7065,11 @@ namespace clang { namespace serialization { SmallVector FactoryMethods; public: - ReadMethodPoolVisitor(ASTReader &Reader, Selector Sel, + ReadMethodPoolVisitor(ASTReader &Reader, Selector Sel, unsigned PriorGeneration) - : Reader(Reader), Sel(Sel), PriorGeneration(PriorGeneration), - InstanceBits(0), FactoryBits(0) { } - + : Reader(Reader), Sel(Sel), PriorGeneration(PriorGeneration), + InstanceBits(0), FactoryBits(0) {} + static bool visit(ModuleFile &M, void *UserData) { ReadMethodPoolVisitor *This = static_cast(UserData); diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 02663ad43d43..c5d5b984cd39 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -2895,11 +2895,11 @@ public: unsigned DataLen = 4 + 2 + 2; // 2 bytes for each of the method counts for (const ObjCMethodList *Method = &Methods.Instance; Method; Method = Method->getNext()) - if (Method->Method) + if (Method->getMethod()) DataLen += 4; for (const ObjCMethodList *Method = &Methods.Factory; Method; Method = Method->getNext()) - if (Method->Method) + if (Method->getMethod()) DataLen += 4; LE.write(DataLen); return std::make_pair(KeyLen, DataLen); @@ -2929,13 +2929,13 @@ public: unsigned NumInstanceMethods = 0; for (const ObjCMethodList *Method = &Methods.Instance; Method; Method = Method->getNext()) - if (Method->Method) + if (Method->getMethod()) ++NumInstanceMethods; unsigned NumFactoryMethods = 0; for (const ObjCMethodList *Method = &Methods.Factory; Method; Method = Method->getNext()) - if (Method->Method) + if (Method->getMethod()) ++NumFactoryMethods; unsigned InstanceBits = Methods.Instance.getBits(); @@ -2949,12 +2949,12 @@ public: LE.write(NumFactoryMethodsAndBits); for (const ObjCMethodList *Method = &Methods.Instance; Method; Method = Method->getNext()) - if (Method->Method) - LE.write(Writer.getDeclID(Method->Method)); + if (Method->getMethod()) + LE.write(Writer.getDeclID(Method->getMethod())); for (const ObjCMethodList *Method = &Methods.Factory; Method; Method = Method->getNext()) - if (Method->Method) - LE.write(Writer.getDeclID(Method->Method)); + if (Method->getMethod()) + LE.write(Writer.getDeclID(Method->getMethod())); assert(Out.tell() - Start == DataLen && "Data length is wrong"); } @@ -3000,19 +3000,19 @@ void ASTWriter::WriteSelectors(Sema &SemaRef) { if (Chain && I->second < FirstSelectorID) { // Selector already exists. Did it change? bool changed = false; - for (ObjCMethodList *M = &Data.Instance; !changed && M && M->Method; - M = M->getNext()) { - if (!M->Method->isFromASTFile()) + for (ObjCMethodList *M = &Data.Instance; + !changed && M && M->getMethod(); M = M->getNext()) { + if (!M->getMethod()->isFromASTFile()) changed = true; } - for (ObjCMethodList *M = &Data.Factory; !changed && M && M->Method; + for (ObjCMethodList *M = &Data.Factory; !changed && M && M->getMethod(); M = M->getNext()) { - if (!M->Method->isFromASTFile()) + if (!M->getMethod()->isFromASTFile()) changed = true; } if (!changed) continue; - } else if (Data.Instance.Method || Data.Factory.Method) { + } else if (Data.Instance.getMethod() || Data.Factory.getMethod()) { // A new method pool entry. ++NumTableEntries; }