r224892 - Objective-C: Serialize "more than one decl" state of ObjCMethodList.

Nico Weber thakis at chromium.org
Sun Dec 28 13:50:16 PST 2014


On Sat, Dec 27, 2014 at 2:14 PM, Nico Weber <nicolasweber at gmx.de> wrote:

> Author: nico
> Date: Sat Dec 27 16:14:15 2014
> New Revision: 224892
>
> URL: http://llvm.org/viewvc/llvm-project?rev=224892&view=rev
> Log:
> Objective-C: Serialize "more than one decl" state of ObjCMethodList.
>
> This fixes PR21587, what r221933 fixed for regular programs is now also
> fixed for decls coming from PCH files.
>
> Use another bit from the count/bits uint16_t for storing the "more than one
> decl" bit.  This reduces the number of bits for the count from 14 to 13.
> The selector with the most overloads in Cocoa.h has ~55 overloads, so 13
> bits
> should still be plenty.  Since this changes the meaning of a serialized bit
> pattern, also increase clang::serialization::VERSION_MAJOR.
>
> Storing the "more than one decl" state of only the first overload isn't
> quite
> correct, but Sema::AreMultipleMethodsInGlobalPool() currently only looks at
> the state of the first overload so it's good enough for now.
>

I filed http://llvm.org/PR22047 for things that go wrong with the current
approach for this warning (these things go wrong independent of PCH files).


>
> Added:
>     cfe/trunk/test/SemaObjC/attr-deprecated-pch.m
> Modified:
>     cfe/trunk/include/clang/Sema/ObjCMethodList.h
>     cfe/trunk/include/clang/Serialization/ASTBitCodes.h
>     cfe/trunk/lib/Serialization/ASTReader.cpp
>     cfe/trunk/lib/Serialization/ASTReaderInternals.h
>     cfe/trunk/lib/Serialization/ASTWriter.cpp
>
> Modified: cfe/trunk/include/clang/Sema/ObjCMethodList.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/ObjCMethodList.h?rev=224892&r1=224891&r2=224892&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Sema/ObjCMethodList.h (original)
> +++ cfe/trunk/include/clang/Sema/ObjCMethodList.h Sat Dec 27 16:14:15 2014
> @@ -23,6 +23,7 @@ class ObjCMethodDecl;
>  /// \brief a linked list of methods with the same selector name but
> different
>  /// signatures.
>  struct ObjCMethodList {
> +  // NOTE: If you add any members to this struct, make sure to serialize
> them.
>    /// \brief If there is more than one decl with this signature.
>    llvm::PointerIntPair<ObjCMethodDecl *, 1> MethodAndHasMoreThanOneDecl;
>    /// \brief The next list object and 2 bits for extra info.
>
> Modified: cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTBitCodes.h?rev=224892&r1=224891&r2=224892&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Serialization/ASTBitCodes.h (original)
> +++ cfe/trunk/include/clang/Serialization/ASTBitCodes.h Sat Dec 27
> 16:14:15 2014
> @@ -35,7 +35,7 @@ namespace clang {
>      /// Version 4 of AST files also requires that the version control
> branch and
>      /// revision match exactly, since there is no backward compatibility
> of
>      /// AST files at this time.
> -    const unsigned VERSION_MAJOR = 5;
> +    const unsigned VERSION_MAJOR = 6;
>
>      /// \brief AST file minor version number supported by this version of
>      /// Clang.
>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=224892&r1=224891&r2=224892&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Sat Dec 27 16:14:15 2014
> @@ -650,14 +650,14 @@ ASTSelectorLookupTrait::ReadData(Selecto
>
>    Result.ID = Reader.getGlobalSelectorID(
>        F, endian::readNext<uint32_t, little, unaligned>(d));
> -  unsigned NumInstanceMethodsAndBits =
> -      endian::readNext<uint16_t, little, unaligned>(d);
> -  unsigned NumFactoryMethodsAndBits =
> -      endian::readNext<uint16_t, little, unaligned>(d);
> -  Result.InstanceBits = NumInstanceMethodsAndBits & 0x3;
> -  Result.FactoryBits = NumFactoryMethodsAndBits & 0x3;
> -  unsigned NumInstanceMethods = NumInstanceMethodsAndBits >> 2;
> -  unsigned NumFactoryMethods = NumFactoryMethodsAndBits >> 2;
> +  unsigned FullInstanceBits = endian::readNext<uint16_t, little,
> unaligned>(d);
> +  unsigned FullFactoryBits = endian::readNext<uint16_t, little,
> unaligned>(d);
> +  Result.InstanceBits = FullInstanceBits & 0x3;
> +  Result.InstanceHasMoreThanOneDecl = (FullInstanceBits >> 2) & 0x1;
> +  Result.FactoryBits = FullFactoryBits & 0x3;
> +  Result.FactoryHasMoreThanOneDecl = (FullFactoryBits >> 2) & 0x1;
> +  unsigned NumInstanceMethods = FullInstanceBits >> 3;
> +  unsigned NumFactoryMethods = FullFactoryBits >> 3;
>
>    // Load instance methods
>    for (unsigned I = 0; I != NumInstanceMethods; ++I) {
> @@ -7061,6 +7061,8 @@ namespace clang { namespace serializatio
>      unsigned PriorGeneration;
>      unsigned InstanceBits;
>      unsigned FactoryBits;
> +    bool InstanceHasMoreThanOneDecl;
> +    bool FactoryHasMoreThanOneDecl;
>      SmallVector<ObjCMethodDecl *, 4> InstanceMethods;
>      SmallVector<ObjCMethodDecl *, 4> FactoryMethods;
>
> @@ -7068,7 +7070,8 @@ namespace clang { namespace serializatio
>      ReadMethodPoolVisitor(ASTReader &Reader, Selector Sel,
>                            unsigned PriorGeneration)
>          : Reader(Reader), Sel(Sel), PriorGeneration(PriorGeneration),
> -          InstanceBits(0), FactoryBits(0) {}
> +          InstanceBits(0), FactoryBits(0),
> InstanceHasMoreThanOneDecl(false),
> +          FactoryHasMoreThanOneDecl(false) {}
>
>      static bool visit(ModuleFile &M, void *UserData) {
>        ReadMethodPoolVisitor *This
> @@ -7103,6 +7106,8 @@ namespace clang { namespace serializatio
>        This->FactoryMethods.append(Data.Factory.begin(),
> Data.Factory.end());
>        This->InstanceBits = Data.InstanceBits;
>        This->FactoryBits = Data.FactoryBits;
> +      This->InstanceHasMoreThanOneDecl = Data.InstanceHasMoreThanOneDecl;
> +      This->FactoryHasMoreThanOneDecl = Data.FactoryHasMoreThanOneDecl;
>        return true;
>      }
>
> @@ -7118,6 +7123,10 @@ namespace clang { namespace serializatio
>
>      unsigned getInstanceBits() const { return InstanceBits; }
>      unsigned getFactoryBits() const { return FactoryBits; }
> +    bool instanceHasMoreThanOneDecl() const {
> +      return InstanceHasMoreThanOneDecl;
> +    }
> +    bool factoryHasMoreThanOneDecl() const { return
> FactoryHasMoreThanOneDecl; }
>    };
>  } } // end namespace clang::serialization
>
> @@ -7156,7 +7165,9 @@ void ASTReader::ReadMethodPool(Selector
>    addMethodsToPool(S, Visitor.getInstanceMethods(), Pos->second.first);
>    addMethodsToPool(S, Visitor.getFactoryMethods(), Pos->second.second);
>    Pos->second.first.setBits(Visitor.getInstanceBits());
> +
> Pos->second.first.setHasMoreThanOneDecl(Visitor.instanceHasMoreThanOneDecl());
>    Pos->second.second.setBits(Visitor.getFactoryBits());
> +
> Pos->second.second.setHasMoreThanOneDecl(Visitor.factoryHasMoreThanOneDecl());
>  }
>
>  void ASTReader::ReadKnownNamespaces(
>
> Modified: cfe/trunk/lib/Serialization/ASTReaderInternals.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderInternals.h?rev=224892&r1=224891&r2=224892&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReaderInternals.h (original)
> +++ cfe/trunk/lib/Serialization/ASTReaderInternals.h Sat Dec 27 16:14:15
> 2014
> @@ -156,6 +156,8 @@ public:
>      SelectorID ID;
>      unsigned InstanceBits;
>      unsigned FactoryBits;
> +    bool InstanceHasMoreThanOneDecl;
> +    bool FactoryHasMoreThanOneDecl;
>      SmallVector<ObjCMethodDecl *, 2> Instance;
>      SmallVector<ObjCMethodDecl *, 2> Factory;
>    };
>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=224892&r1=224891&r2=224892&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Sat Dec 27 16:14:15 2014
> @@ -2940,13 +2940,20 @@ public:
>
>      unsigned InstanceBits = Methods.Instance.getBits();
>      assert(InstanceBits < 4);
> -    unsigned NumInstanceMethodsAndBits =
> -        (NumInstanceMethods << 2) | InstanceBits;
> +    unsigned InstanceHasMoreThanOneDeclBit =
> +        Methods.Instance.hasMoreThanOneDecl();
> +    unsigned FullInstanceBits = (NumInstanceMethods << 3) |
> +                                (InstanceHasMoreThanOneDeclBit << 2) |
> +                                InstanceBits;
>      unsigned FactoryBits = Methods.Factory.getBits();
>      assert(FactoryBits < 4);
> -    unsigned NumFactoryMethodsAndBits = (NumFactoryMethods << 2) |
> FactoryBits;
> -    LE.write<uint16_t>(NumInstanceMethodsAndBits);
> -    LE.write<uint16_t>(NumFactoryMethodsAndBits);
> +    unsigned FactoryHasMoreThanOneDeclBit =
> +        Methods.Factory.hasMoreThanOneDecl();
> +    unsigned FullFactoryBits = (NumFactoryMethods << 3) |
> +                               (FactoryHasMoreThanOneDeclBit << 2) |
> +                               FactoryBits;
> +    LE.write<uint16_t>(FullInstanceBits);
> +    LE.write<uint16_t>(FullFactoryBits);
>      for (const ObjCMethodList *Method = &Methods.Instance; Method;
>           Method = Method->getNext())
>        if (Method->getMethod())
>
> Added: cfe/trunk/test/SemaObjC/attr-deprecated-pch.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/attr-deprecated-pch.m?rev=224892&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/SemaObjC/attr-deprecated-pch.m (added)
> +++ cfe/trunk/test/SemaObjC/attr-deprecated-pch.m Sat Dec 27 16:14:15 2014
> @@ -0,0 +1,23 @@
> +// RUN: %clang_cc1 -fsyntax-only -DBOTH -verify %s
> +// If the decls come from a pch, the behavior shouldn't change:
> +// RUN: %clang_cc1 -x objective-c-header %s -emit-pch -o %t
> +// RUN: %clang_cc1 -DUSES -include-pch %t -fsyntax-only -verify %s
> +// expected-no-diagnostics
> +
> +// The slightly strange ifdefs are so that the command that builds the
> gch file
> +// doesn't need any -D switches, for these would get embedded in the gch.
> +
> +#ifndef USES
> + at interface Interface1
> +- (void)partiallyUnavailableMethod;
> + at end
> + at interface Interface2
> +- (void)partiallyUnavailableMethod __attribute__((unavailable));
> + at end
> +#endif
> +
> +#if defined(USES) || defined(BOTH)
> +void f(id a) {
> +  [a partiallyUnavailableMethod];  // no warning, `a` could be an
> Interface1.
> +}
> +#endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20141228/1ddc3000/attachment.html>


More information about the cfe-commits mailing list