[cfe-commits] r126817 - in /cfe/trunk: include/clang/AST/DeclObjC.h include/clang/AST/ExprObjC.h include/clang/Analysis/DomainSpecific/CocoaConventions.h include/clang/Basic/IdentifierTable.h include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMe

[Matt Beaumont-Gay]

Hi John,

On Tue, Mar 1, 2011 at 17:50, John McCall <rjmccall at apple.com> wrote:
> Author: rjmccall
> Date: Tue Mar  1 19:50:55 2011
> New Revision: 126817
>
> URL: http://llvm.org/viewvc/llvm-project?rev=126817&view=rev
> Log:
> Move some of the logic about classifying Objective-C methods into
> conventional categories into Basic and AST.  Update the self-init checker
> to use this logic;  CFRefCountChecker is complicated enough that I didn't
> want to touch it.
>
>
> Modified:
>    cfe/trunk/include/clang/AST/DeclObjC.h
>    cfe/trunk/include/clang/AST/ExprObjC.h
>    cfe/trunk/include/clang/Analysis/DomainSpecific/CocoaConventions.h
>    cfe/trunk/include/clang/Basic/IdentifierTable.h
>    cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/ObjCMessage.h
>    cfe/trunk/lib/AST/DeclObjC.cpp
>    cfe/trunk/lib/Analysis/CocoaConventions.cpp
>    cfe/trunk/lib/Basic/IdentifierTable.cpp
>    cfe/trunk/lib/StaticAnalyzer/Checkers/ObjCSelfInitChecker.cpp
>    cfe/trunk/lib/StaticAnalyzer/Core/ObjCMessage.cpp
>
> Modified: cfe/trunk/include/clang/AST/DeclObjC.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/DeclObjC.h?rev=126817&r1=126816&r2=126817&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/DeclObjC.h (original)
> +++ cfe/trunk/include/clang/AST/DeclObjC.h Tue Mar  1 19:50:55 2011
> @@ -112,17 +112,20 @@
>  public:
>   enum ImplementationControl { None, Required, Optional };
>  private:
> -  /// Bitfields must be first fields in this class so they pack with those
> -  /// declared in class Decl.
> +  // The conventional meaning of this method; an ObjCMethodFamily.
> +  // This is not serialized; instead, it is computed on demand and
> +  // cached.
> +  mutable unsigned Family : ObjCMethodFamilyBitWidth;
> +
>   /// instance (true) or class (false) method.
> -  bool IsInstance : 1;
> -  bool IsVariadic : 1;
> +  unsigned IsInstance : 1;
> +  unsigned IsVariadic : 1;
>
>   // Synthesized declaration method for a property setter/getter
> -  bool IsSynthesized : 1;
> +  unsigned IsSynthesized : 1;
>
>   // Method has a definition.
> -  bool IsDefined : 1;
> +  unsigned IsDefined : 1;
>
>   // NOTE: VC++ treats enums as signed, avoid using ImplementationControl enum
>   /// @required/@optional
> @@ -170,7 +173,7 @@
>                  ImplementationControl impControl = None,
>                  unsigned numSelectorArgs = 0)
>   : NamedDecl(ObjCMethod, contextDecl, beginLoc, SelInfo),
> -    DeclContext(ObjCMethod),
> +    DeclContext(ObjCMethod), Family(InvalidObjCMethodFamily),
>     IsInstance(isInstance), IsVariadic(isVariadic),
>     IsSynthesized(isSynthesized),
>     IsDefined(isDefined),
> @@ -279,6 +282,9 @@
>   ImplicitParamDecl * getCmdDecl() const { return CmdDecl; }
>   void setCmdDecl(ImplicitParamDecl *CD) { CmdDecl = CD; }
>
> +  /// Determines the family of this method.
> +  ObjCMethodFamily getMethodFamily() const;
> +
>   bool isInstanceMethod() const { return IsInstance; }
>   void setInstanceMethod(bool isInst) { IsInstance = isInst; }
>   bool isVariadic() const { return IsVariadic; }
>
> Modified: cfe/trunk/include/clang/AST/ExprObjC.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ExprObjC.h?rev=126817&r1=126816&r2=126817&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/ExprObjC.h (original)
> +++ cfe/trunk/include/clang/AST/ExprObjC.h Tue Mar  1 19:50:55 2011
> @@ -741,6 +741,11 @@
>     SelectorOrMethod = reinterpret_cast<uintptr_t>(MD);
>   }
>
> +  ObjCMethodFamily getMethodFamily() const {
> +    if (HasMethod) return getMethodDecl()->getMethodFamily();
> +    return getSelector().getMethodFamily();
> +  }
> +
>   /// \brief Return the number of actual arguments in this message,
>   /// not counting the receiver.
>   unsigned getNumArgs() const { return NumArgs; }
>
> Modified: cfe/trunk/include/clang/Analysis/DomainSpecific/CocoaConventions.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/DomainSpecific/CocoaConventions.h?rev=126817&r1=126816&r2=126817&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Analysis/DomainSpecific/CocoaConventions.h (original)
> +++ cfe/trunk/include/clang/Analysis/DomainSpecific/CocoaConventions.h Tue Mar  1 19:50:55 2011
> @@ -22,7 +22,7 @@
>
>   enum NamingConvention { NoConvention, CreateRule, InitRule };
>
> -  NamingConvention deriveNamingConvention(Selector S, bool ignorePrefix = true);
> +  NamingConvention deriveNamingConvention(Selector S);
>
>   static inline bool followsFundamentalRule(Selector S) {
>     return deriveNamingConvention(S) == CreateRule;
>
> Modified: cfe/trunk/include/clang/Basic/IdentifierTable.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/IdentifierTable.h?rev=126817&r1=126816&r2=126817&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/IdentifierTable.h (original)
> +++ cfe/trunk/include/clang/Basic/IdentifierTable.h Tue Mar  1 19:50:55 2011
> @@ -443,6 +443,52 @@
>   void AddKeywords(const LangOptions &LangOpts);
>  };
>
> +/// ObjCMethodFamily - A family of Objective-C methods.  These
> +/// families have no inherent meaning in the language, but are
> +/// nonetheless central enough in the existing implementations to
> +/// merit direct AST support.  While, in theory, arbitrary methods can
> +/// be considered to form families, we focus here on the methods
> +/// involving allocation and retain-count management, as these are the
> +/// most "core" and the most likely to be useful to diverse clients
> +/// without extra information.
> +///
> +/// Both selectors and actual method declarations may be classified
> +/// into families.  Method families may impose additional restrictions
> +/// beyond their selector name; for example, a method called '_init'
> +/// that returns void is not considered to be in the 'init' family
> +/// (but would be if it returned 'id').  It is also possible to
> +/// explicitly change or remove a method's family.  Therefore the
> +/// method's family should be considered the single source of truth.
> +enum ObjCMethodFamily {
> +  /// \brief No particular method family.
> +  OMF_None,
> +
> +  // Selectors in these families may have arbitrary arity, may be
> +  // written with arbitrary leading underscores, and may have
> +  // additional CamelCase "words" in their first selector chunk
> +  // following the family name.
> +  OMF_alloc,
> +  OMF_copy,
> +  OMF_init,
> +  OMF_mutableCopy,
> +  OMF_new,
> +
> +  // These families are singletons consisting only of the nullary
> +  // selector with the given name.
> +  OMF_autorelease,
> +  OMF_dealloc,
> +  OMF_release,
> +  OMF_retain,
> +  OMF_retainCount
> +};
> +
> +/// Enough bits to store any enumerator in ObjCMethodFamily or
> +/// InvalidObjCMethodFamily.
> +enum { ObjCMethodFamilyBitWidth = 4 };
> +
> +/// An invalid value of ObjCMethodFamily.
> +enum { InvalidObjCMethodFamily = (1 << ObjCMethodFamilyBitWidth) - 1 };

Unfortunately, GCC's -Wenum-compare doesn't ignore anonymous enums
like Clang's does ("tools/clang/lib/AST/DeclObjC.cpp: 403: error:
comparison between 'enum clang::ObjCMethodFamily' and 'enum
clang::<anonymous>' [-Wenum-compare]"). Would it be OK to make
InvalidObjCMethodFamily part of the ObjCMethodFamily enum?

-Matt