r195323 - Add new attribute 'objc_suppress_protocol' to suppress protocol conformance for a class.
Aaron Ballman
aaron at aaronballman.com
Thu Nov 21 05:27:46 PST 2013
I know you already reverted this, but I still have a few comments for
when you bring this back.
On Thu, Nov 21, 2013 at 2:20 AM, Ted Kremenek <kremenek at apple.com> wrote:
> Author: kremenek
> Date: Thu Nov 21 01:20:42 2013
> New Revision: 195323
>
> URL: http://llvm.org/viewvc/llvm-project?rev=195323&view=rev
> Log:
> Add new attribute 'objc_suppress_protocol' to suppress protocol conformance for a class.
>
> The idea is to allow a class to stipulate that its methods (and those
> of its parents) cannot be used for protocol conformance in a subclass.
> A subclass is then explicitly required to re-implement those methods
> of they are present in the class marked with this attribute.
>
> Currently the attribute can only be applied to an @interface, and
> not a category or class extension. This is by design. Unlike
> protocol conformance, where a category can add explicit conformance
> of a protocol to class, this anti-conformance really needs to be
> observed uniformly by all clients of the class. That's because
> the absence of the attribute implies more permissive checking of
> protocol conformance.
>
> This unfortunately required changing method lookup in ObjCInterfaceDecl
> to take an optional protocol parameter. This should not slow down
> method lookup in most cases, and is just used for protocol conformance.
>
> Added:
> cfe/trunk/test/SemaObjC/protocols-suppress-conformance.m
> Modified:
> cfe/trunk/include/clang/AST/DeclObjC.h
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/lib/AST/DeclObjC.cpp
> cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> cfe/trunk/lib/Sema/SemaDeclObjC.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclObjC.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=195323&r1=195322&r2=195323&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclObjC.h (original)
> +++ cfe/trunk/include/clang/AST/DeclObjC.h Thu Nov 21 01:20:42 2013
> @@ -1142,14 +1142,17 @@ public:
> // found, we search referenced protocols and class categories.
> ObjCMethodDecl *lookupMethod(Selector Sel, bool isInstance,
> bool shallowCategoryLookup= false,
> - const ObjCCategoryDecl *C= 0) const;
> + const ObjCCategoryDecl *C = 0,
> + const ObjCProtocolDecl *P = 0) const;
> ObjCMethodDecl *lookupInstanceMethod(Selector Sel,
> - bool shallowCategoryLookup = false) const {
> - return lookupMethod(Sel, true/*isInstance*/, shallowCategoryLookup);
> + bool shallowCategoryLookup = false,
> + ObjCProtocolDecl *P = 0) const {
> + return lookupMethod(Sel, true/*isInstance*/, shallowCategoryLookup, 0, P);
> }
> ObjCMethodDecl *lookupClassMethod(Selector Sel,
> - bool shallowCategoryLookup = false) const {
> - return lookupMethod(Sel, false/*isInstance*/, shallowCategoryLookup);
> + bool shallowCategoryLookup = false,
> + ObjCProtocolDecl *P = 0) const {
> + return lookupMethod(Sel, false/*isInstance*/, shallowCategoryLookup, 0, P);
> }
> ObjCInterfaceDecl *lookupInheritedClass(const IdentifierInfo *ICName);
>
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=195323&r1=195322&r2=195323&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Thu Nov 21 01:20:42 2013
> @@ -613,6 +613,12 @@ def ObjCRootClass : InheritableAttr {
> let Subjects = [ObjCInterface];
> }
>
> +def ObjCSuppressProtocol : InheritableAttr {
> + let Spellings = [GNU<"objc_suppress_protocol">];
> + let Subjects = [ObjCInterface];
> + let Args = [IdentifierArgument<"Protocol", 1>];
This arg doesn't appear to actually be optional, so you can strip the
,1 from it.
> +}
> +
> def Overloadable : Attr {
> let Spellings = [GNU<"overloadable">];
> }
>
> Modified: cfe/trunk/lib/AST/DeclObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/DeclObjC.cpp?rev=195323&r1=195322&r2=195323&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/DeclObjC.cpp (original)
> +++ cfe/trunk/lib/AST/DeclObjC.cpp Thu Nov 21 01:20:42 2013
> @@ -457,9 +457,11 @@ ObjCInterfaceDecl::lookupNestedProtocol(
> /// When argument category "C" is specified, any implicit method found
> /// in this category is ignored.
> ObjCMethodDecl *ObjCInterfaceDecl::lookupMethod(Selector Sel,
> - bool isInstance,
> - bool shallowCategoryLookup,
> - const ObjCCategoryDecl *C) const {
> + bool isInstance,
> + bool shallowCategoryLookup,
> + const ObjCCategoryDecl *C,
> + const ObjCProtocolDecl *P) const
> +{
> // FIXME: Should make sure no callers ever do this.
> if (!hasDefinition())
> return 0;
> @@ -470,7 +472,23 @@ ObjCMethodDecl *ObjCInterfaceDecl::looku
> if (data().ExternallyCompleted)
> LoadExternalDefinition();
>
> - while (ClassDecl != NULL) {
> + while (ClassDecl) {
> + // If we are looking for a method that is part of protocol conformance,
> + // check if the class has been marked to suppress conformance
> + // of that protocol.
> + if (P && ClassDecl->hasAttrs()) {
> + const AttrVec &V = ClassDecl->getAttrs();
> + const IdentifierInfo *PI = P->getIdentifier();
> + for (AttrVec::const_iterator I = V.begin(), E = V.end(); I != E; ++I) {
> + if (const ObjCSuppressProtocolAttr *A =
> + dyn_cast<ObjCSuppressProtocolAttr>(*I)){
> + if (A->getProtocol() == PI) {
> + return 0;
> + }
> + }
> + }
> + }
> +
> if ((MethodDecl = ClassDecl->getMethod(Sel, isInstance)))
> return MethodDecl;
>
>
> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=195323&r1=195322&r2=195323&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Thu Nov 21 01:20:42 2013
> @@ -2134,6 +2134,30 @@ static void handleObjCRootClassAttr(Sema
> Attr.getAttributeSpellingListIndex()));
> }
>
> +static void handleObjCSuppresProtocolAttr(Sema &S, Decl *D,
> + const AttributeList &Attr) {
> + if (!isa<ObjCInterfaceDecl>(D)) {
> + S.Diag(Attr.getLoc(), diag::err_attribute_wrong_decl_type)
> + << Attr.getName() << ExpectedObjectiveCInterface;
> + return;
> + }
Missing a test case for this.
> +
> + IdentifierLoc *Parm = NULL;
> + if (Attr.getNumArgs() == 1) {
> + Parm = Attr.isArgIdent(0) ? Attr.getArgAsIdent(0) : 0;
> + }
> +
> + if (!Parm) {
> + S.Diag(D->getLocStart(), diag::err_objc_attr_not_id) << Attr.getName() << 1;
> + return;
> + }
These checks can be simplified once you remove the ,1 from Attr.td --
the common attribute checker will already bail out if there are no
args passed, so you can simply do:
if (!Attr.isArgIdent(0)) {
S.Diag(Attr.getLoc(), diag::err_attribute_argument_type) << Attr.getName()
<< AANT_ArgumentIdentifier;
return;
}
IdentifierInfo *Parm = Attr.getArgAsIdent(0)->Ident;
You should also add a test case for the number and type of args passed in.
As for the err_objc_attr_not_id logic, you should actually be checking
that instead of assuming any identifier counts, and this logic should
have a test case.
> +
> + D->addAttr(::new (S.Context)
> + ObjCSuppressProtocolAttr(Attr.getRange(), S.Context, Parm->Ident,
> + Attr.getAttributeSpellingListIndex()));
> +}
> +
> +
> static void handleObjCRequiresPropertyDefsAttr(Sema &S, Decl *D,
> const AttributeList &Attr) {
> if (!isa<ObjCInterfaceDecl>(D)) {
> @@ -4690,7 +4714,10 @@ static void ProcessDeclAttribute(Sema &S
> case AttributeList::AT_ObjCRootClass:
> handleObjCRootClassAttr(S, D, Attr);
> break;
> - case AttributeList::AT_ObjCRequiresPropertyDefs:
> + case AttributeList::AT_ObjCSuppressProtocol:
> + handleObjCSuppresProtocolAttr(S, D, Attr);
> + break;
> + case AttributeList::AT_ObjCRequiresPropertyDefs:
> handleObjCRequiresPropertyDefsAttr (S, D, Attr);
> break;
> case AttributeList::AT_Unused: handleUnusedAttr (S, D, Attr); break;
>
> Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=195323&r1=195322&r2=195323&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Thu Nov 21 01:20:42 2013
> @@ -1664,7 +1664,8 @@ void Sema::CheckProtocolMethodDefs(Sourc
> if (method->getImplementationControl() != ObjCMethodDecl::Optional &&
> !method->isPropertyAccessor() &&
> !InsMap.count(method->getSelector()) &&
> - (!Super || !Super->lookupInstanceMethod(method->getSelector()))) {
> + (!Super || !Super->lookupInstanceMethod(method->getSelector(),
> + false, PDecl))) {
> // If a method is not implemented in the category implementation but
> // has been declared in its primary class, superclass,
> // or in one of their protocols, no need to issue the warning.
> @@ -1676,7 +1677,8 @@ void Sema::CheckProtocolMethodDefs(Sourc
> // uses the protocol.
> if (ObjCMethodDecl *MethodInClass =
> IDecl->lookupInstanceMethod(method->getSelector(),
> - true /*shallowCategoryLookup*/))
> + true /*shallowCategoryLookup*/,
> + PDecl))
> if (C || MethodInClass->isPropertyAccessor())
> continue;
> unsigned DIAG = diag::warn_unimplemented_protocol_method;
> @@ -1695,10 +1697,13 @@ void Sema::CheckProtocolMethodDefs(Sourc
> ObjCMethodDecl *method = *I;
> if (method->getImplementationControl() != ObjCMethodDecl::Optional &&
> !ClsMap.count(method->getSelector()) &&
> - (!Super || !Super->lookupClassMethod(method->getSelector()))) {
> + (!Super || !Super->lookupClassMethod(method->getSelector(),
> + /* shallowCategoryLookup */ false,
> + PDecl))) {
> // See above comment for instance method lookups.
> if (C && IDecl->lookupClassMethod(method->getSelector(),
> - true /*shallowCategoryLookup*/))
> + true /*shallowCategoryLookup*/,
> + PDecl))
> continue;
> unsigned DIAG = diag::warn_unimplemented_protocol_method;
> if (Diags.getDiagnosticLevel(DIAG, ImpLoc) !=
>
> Added: cfe/trunk/test/SemaObjC/protocols-suppress-conformance.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/protocols-suppress-conformance.m?rev=195323&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/protocols-suppress-conformance.m (added)
> +++ cfe/trunk/test/SemaObjC/protocols-suppress-conformance.m Thu Nov 21 01:20:42 2013
> @@ -0,0 +1,27 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s -Wno-objc-root-class
> +
> + at protocol FooProto
> +- (void) theBestOfTimes; // expected-note {{method 'theBestOfTimes' declared here}}
> + at end
> +
> +__attribute__((objc_suppress_protocol(FooProto)))
> + at interface Bar
> +- (void) theBestOfTimes;
> + at end
> +
> + at interface Bar2 : Bar
> + at end
> +
> + at interface Baz : Bar2 <FooProto> // expected-note {{required for direct or indirect protocol 'FooProto'}}
> + at end
> +
> + at interface Baz2 : Bar2 <FooProto>
> +- (void) theBestOfTimes;
> + at end
> +
> + at implementation Baz // expected-warning {{method 'theBestOfTimes' in protocol not implemented}}
> + at end
> +
> + at implementation Baz2 // no-warning
> +- (void) theBestOfTimes {}
> + at end
> \ No newline at end of file
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
~Aaron
More information about the cfe-commits
mailing list