[cfe-commits] r139989 - in /cfe/trunk: include/clang/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaExprObjC.cpp test/SemaObjC/class-type-conversion.m
Douglas Gregor
dgregor at apple.com
Mon Sep 19 07:50:40 PDT 2011
On Sep 17, 2011, at 12:23 PM, Fariborz Jahanian wrote:
> Author: fjahanian
> Date: Sat Sep 17 14:23:40 2011
> New Revision: 139989
>
> URL: http://llvm.org/viewvc/llvm-project?rev=139989&view=rev
> Log:
> objc - Treat type of 'self' in class methods as root of
> class of this method. // rdar://10109725
I'm still quite dubious about the semantics behind this change, because it seems very NeXT-runtime centric, but we can take discussion that off-line. Another comment below…
> Added:
> cfe/trunk/test/SemaObjC/class-type-conversion.m
> Modified:
> cfe/trunk/include/clang/Sema/Sema.h
> cfe/trunk/lib/Sema/SemaExpr.cpp
> cfe/trunk/lib/Sema/SemaExprObjC.cpp
>
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=139989&r1=139988&r2=139989&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Sat Sep 17 14:23:40 2011
> @@ -675,8 +675,9 @@
>
> GlobalMethodPool::iterator ReadMethodPool(Selector Sel);
>
> - /// Private Helper predicate to check for 'self'.
> - bool isSelfExpr(Expr *RExpr);
> + /// Private Helper predicate to check for 'self'. Upon success, it
> + /// returns method declaration where 'self' is referenced.
> + const ObjCMethodDecl *GetMethodIfSelfExpr(Expr *RExpr);
> public:
> Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer,
> TranslationUnitKind TUKind = TU_Complete,
>
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=139989&r1=139988&r2=139989&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Sat Sep 17 14:23:40 2011
> @@ -5074,6 +5074,23 @@
> OK));
> }
>
> +/// SelfInClassMethodType - convet type of 'self' in class method
> +/// to pointer to root of method's class.
> +static void
> +SelfInClassMethodType(Sema &S, Expr *selfExpr, QualType &SelfType) {
> + if (const ObjCMethodDecl *MD = S.GetMethodIfSelfExpr(selfExpr))
> + if (MD->isClassMethod()) {
> + const ObjCInterfaceDecl *Root = 0;
> + if (const ObjCInterfaceDecl * IDecl = MD->getClassInterface())
> + do {
> + Root = IDecl;
> + } while ((IDecl = IDecl->getSuperClass()));
> + if (Root)
> + SelfType = S.Context.getObjCObjectPointerType(
> + S.Context.getObjCInterfaceType(Root));
> + }
> +}
I suggest returning a QualType with the requested result; it's cleaner than modifying the parameter. Also, can the function name say something about returning the root? That's not at all apparent from the interface (nor is it particularly intuitive).
> // checkPointerTypesForAssignment - This is a very tricky routine (despite
> // being closely modeled after the C99 spec:-). The odd characteristic of this
> // routine is it effectively iqnores the qualifiers on the top level pointee.
> @@ -5309,6 +5326,8 @@
> return Compatible;
> }
>
> + SelfInClassMethodType(*this, RHS.get(), RHSType);
I'm not a big fan of speculatively modifying the RHSType here, because that has an impact on *all* of the checking that follows. This special conversion should be checked in isolation, down where we're checking the other Objective-C conversions.
> // If the left-hand side is a reference type, then we are in a
> // (rare!) case where we've allowed the use of references in C,
> // e.g., as a parameter type in a built-in function. In this case,
> @@ -9564,7 +9583,7 @@
> Selector Sel = ME->getSelector();
>
> // self = [<foo> init...]
> - if (isSelfExpr(Op->getLHS()) && Sel.getNameForSlot(0).startswith("init"))
> + if (GetMethodIfSelfExpr(Op->getLHS()) && Sel.getNameForSlot(0).startswith("init"))
> diagnostic = diag::warn_condition_is_idiomatic_assignment;
>
> // <foo> = [<bar> nextObject]
>
> Modified: cfe/trunk/lib/Sema/SemaExprObjC.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprObjC.cpp?rev=139989&r1=139988&r2=139989&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExprObjC.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprObjC.cpp Sat Sep 17 14:23:40 2011
> @@ -457,18 +457,20 @@
> return IsError;
> }
>
> -bool Sema::isSelfExpr(Expr *receiver) {
> +/// GetMethodIfSelfExpr - Check if receiver is an objc 'self' expression
> +/// and return its method declaration if so; else return 0.
> +const ObjCMethodDecl *Sema::GetMethodIfSelfExpr(Expr *receiver) {
> // 'self' is objc 'self' in an objc method only.
> DeclContext *DC = CurContext;
> while (isa<BlockDecl>(DC))
> DC = DC->getParent();
> if (DC && !isa<ObjCMethodDecl>(DC))
> - return false;
> + return 0;
> receiver = receiver->IgnoreParenLValueCasts();
> if (DeclRefExpr *DRE = dyn_cast<DeclRefExpr>(receiver))
> if (DRE->getDecl()->getIdentifier() == &Context.Idents.get("self"))
> - return true;
> - return false;
> + return static_cast<ObjCMethodDecl*>(DC);
> + return 0;
> }
>
> // Helper method for ActOnClassMethod/ActOnInstanceMethod.
> @@ -1272,7 +1274,7 @@
> }
> if (!Method) {
> // If not messaging 'self', look for any factory method named 'Sel'.
> - if (!Receiver || !isSelfExpr(Receiver)) {
> + if (!Receiver || !GetMethodIfSelfExpr(Receiver)) {
> Method = LookupFactoryMethodInGlobalPool(Sel,
> SourceRange(LBracLoc, RBracLoc),
> true);
> @@ -1336,7 +1338,7 @@
> return ExprError();
> }
>
> - if (!Method && (!Receiver || !isSelfExpr(Receiver))) {
> + if (!Method && (!Receiver || !GetMethodIfSelfExpr(Receiver))) {
> // If we still haven't found a method, look in the global pool. This
> // behavior isn't very desirable, however we need it for GCC
> // compatibility. FIXME: should we deviate??
> @@ -1507,7 +1509,7 @@
> if (getLangOptions().ObjCAutoRefCount) {
> // In ARC, annotate delegate init calls.
> if (Result->getMethodFamily() == OMF_init &&
> - (SuperLoc.isValid() || isSelfExpr(Receiver))) {
> + (SuperLoc.isValid() || GetMethodIfSelfExpr(Receiver))) {
> // Only consider init calls *directly* in init implementations,
> // not within blocks.
> ObjCMethodDecl *method = dyn_cast<ObjCMethodDecl>(CurContext);
>
> Added: cfe/trunk/test/SemaObjC/class-type-conversion.m
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/class-type-conversion.m?rev=139989&view=auto
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/class-type-conversion.m (added)
> +++ cfe/trunk/test/SemaObjC/class-type-conversion.m Sat Sep 17 14:23:40 2011
> @@ -0,0 +1,44 @@
> +// RUN: %clang_cc1 -fsyntax-only -verify %s
> +// rdar://10109725
> +
> + at interface NSObject {
> + Class isa;
> +}
> +- (id)addObserver:(NSObject *)observer; // expected-note 2 {{passing argument to parameter 'observer' here}}
> + at end
> +
> + at interface MyClass : NSObject {
> +}
> + at end
> +
> + at implementation NSObject
> ++ (void)initialize
> +{
> + NSObject *obj = 0;
> + [obj addObserver:self];
> +}
> +
> +- init
> +{
> + NSObject *obj = 0;
> + [obj addObserver:self];
> + return [obj addObserver:(Class)0]; // expected-warning {{incompatible pointer types sending 'Class' to parameter of type 'NSObject *'}}
> +}
> +- (id)addObserver:(NSObject *)observer { return 0; }
> + at end
> +
> + at implementation MyClass
> +
> ++ (void)initialize
> +{
> + NSObject *obj = 0;
> + [obj addObserver:self];
> +}
> +
> +- init
> +{
> + NSObject *obj = 0;
> + [obj addObserver:self];
> + return [obj addObserver:(Class)0]; // expected-warning {{incompatible pointer types sending 'Class' to parameter of type 'NSObject *'}}
> +}
> + at end
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list