diff --git a/include/clang/APINotes/APINotesReader.h b/include/clang/APINotes/APINotesReader.h index 29dcc1f56f9..cc6dafb863f 100644 --- a/include/clang/APINotes/APINotesReader.h +++ b/include/clang/APINotes/APINotesReader.h @@ -74,10 +74,13 @@ class APINotesReader { /// /// \param contextID The ID that references the context we are looking for. /// \param name The name of the property we're looking for. + /// \param isInstance Whether we are looking for an instance property (vs. + /// a class property). /// /// \returns Information about the property, if known. Optional lookupObjCProperty(ContextID contextID, - StringRef name); + StringRef name, + bool isInstance); /// Look for information regarding the given Objective-C method in /// the given context. @@ -147,6 +150,7 @@ class APINotesReader { /// Visit an Objective-C property. virtual void visitObjCProperty(ContextID contextID, StringRef name, + bool isInstance, const ObjCPropertyInfo &info); /// Visit a global variable. diff --git a/include/clang/APINotes/APINotesWriter.h b/include/clang/APINotes/APINotesWriter.h index aa41756b2e7..cc93a5f5899 100644 --- a/include/clang/APINotes/APINotesWriter.h +++ b/include/clang/APINotes/APINotesWriter.h @@ -66,6 +66,7 @@ class APINotesWriter { /// \param name The name of this property. /// \param info Information about this property. void addObjCProperty(ContextID contextID, StringRef name, + bool isInstanceProperty, const ObjCPropertyInfo &info); /// Add information about a specific Objective-C method. diff --git a/lib/APINotes/APINotesFormat.h b/lib/APINotes/APINotesFormat.h index aad13f76126..8e5210cc319 100644 --- a/lib/APINotes/APINotesFormat.h +++ b/lib/APINotes/APINotesFormat.h @@ -36,7 +36,7 @@ const uint16_t VERSION_MAJOR = 0; /// API notes file minor version number. /// /// When the format changes IN ANY WAY, this number should be incremented. -const uint16_t VERSION_MINOR = 13; // Function/method parameters +const uint16_t VERSION_MINOR = 14; // Objective-C class properties using IdentifierID = PointerEmbeddedInt; using IdentifierIDField = BCVBR<16>; @@ -291,4 +291,6 @@ namespace llvm { }; } +} + #endif // LLVM_CLANG_API_NOTES_FORMAT_H diff --git a/lib/APINotes/APINotesReader.cpp b/lib/APINotes/APINotesReader.cpp index 56c782a8c9e..2b49125fffc 100644 --- a/lib/APINotes/APINotesReader.cpp +++ b/lib/APINotes/APINotesReader.cpp @@ -175,8 +175,8 @@ namespace { /// Used to deserialize the on-disk Objective-C property table. class ObjCPropertyTableInfo { public: - // (context ID, name ID) - using internal_key_type = std::pair; + // (context ID, name ID, isInstance) + using internal_key_type = std::tuple; using external_key_type = internal_key_type; using data_type = ObjCPropertyInfo; using hash_value_type = size_t; @@ -208,7 +208,8 @@ namespace { static internal_key_type ReadKey(const uint8_t *data, unsigned length) { auto classID = endian::readNext(data); auto nameID = endian::readNext(data); - return { classID, nameID }; + char isInstance = endian::readNext(data); + return std::make_tuple(classID, nameID, isInstance); } static data_type ReadData(internal_key_type key, const uint8_t *data, @@ -1510,7 +1511,8 @@ auto APINotesReader::lookupObjCProtocol(StringRef name) Optional APINotesReader::lookupObjCProperty( ContextID contextID, - StringRef name) { + StringRef name, + bool isInstance) { if (!Impl.ObjCPropertyTable) return None; @@ -1518,7 +1520,9 @@ Optional APINotesReader::lookupObjCProperty( if (!propertyID) return None; - auto known = Impl.ObjCPropertyTable->find({contextID.Value, *propertyID}); + auto known = Impl.ObjCPropertyTable->find(std::make_tuple(contextID.Value, + *propertyID, + (char)isInstance)); if (known == Impl.ObjCPropertyTable->end()) return None; @@ -1639,6 +1643,7 @@ void APINotesReader::Visitor::visitObjCMethod(ContextID contextID, void APINotesReader::Visitor::visitObjCProperty(ContextID contextID, StringRef name, + bool isInstance, const ObjCPropertyInfo &info) { } void APINotesReader::Visitor::visitGlobalVariable( @@ -1723,10 +1728,11 @@ void APINotesReader::visit(Visitor &visitor) { // Visit properties. if (Impl.ObjCPropertyTable) { for (auto key : Impl.ObjCPropertyTable->keys()) { - ContextID contextID(key.first); - auto name = identifiers[key.second]; + ContextID contextID(std::get<0>(key)); + auto name = identifiers[std::get<1>(key)]; + char isInstance = std::get<2>(key); auto info = *Impl.ObjCPropertyTable->find(key); - visitor.visitObjCProperty(contextID, name, info); + visitor.visitObjCProperty(contextID, name, isInstance, info); } } diff --git a/lib/APINotes/APINotesWriter.cpp b/lib/APINotes/APINotesWriter.cpp index 0c178becc16..0e0bfaf1bf4 100644 --- a/lib/APINotes/APINotesWriter.cpp +++ b/lib/APINotes/APINotesWriter.cpp @@ -58,8 +58,9 @@ class APINotesWriter::Implementation { /// Information about Objective-C properties. /// - /// Indexed by the context ID and property name. - llvm::DenseMap, ObjCPropertyInfo> + /// Indexed by the context ID, property name, and whether this is an + /// instance property. + llvm::DenseMap, ObjCPropertyInfo> ObjCProperties; /// Information about Objective-C methods. @@ -430,7 +431,8 @@ namespace { /// Used to serialize the on-disk Objective-C property table. class ObjCPropertyTableInfo { public: - using key_type = std::pair; // (class ID, name ID) + // (class ID, name ID, isInstance) + using key_type = std::tuple; using key_type_ref = key_type; using data_type = ObjCPropertyInfo; using data_type_ref = const data_type &; @@ -444,7 +446,7 @@ namespace { std::pair EmitKeyDataLength(raw_ostream &out, key_type_ref key, data_type_ref data) { - uint32_t keyLength = sizeof(uint32_t) + sizeof(uint32_t); + uint32_t keyLength = sizeof(uint32_t) + sizeof(uint32_t) + sizeof(uint8_t); uint32_t dataLength = getVariableInfoSize(data); endian::Writer writer(out); writer.write(keyLength); @@ -454,8 +456,9 @@ namespace { void EmitKey(raw_ostream &out, key_type_ref key, unsigned len) { endian::Writer writer(out); - writer.write(key.first); - writer.write(key.second); + writer.write(std::get<0>(key)); + writer.write(std::get<1>(key)); + writer.write(std::get<2>(key)); } void EmitData(raw_ostream &out, key_type_ref key, data_type_ref data, @@ -1060,10 +1063,11 @@ ContextID APINotesWriter::addObjCProtocol(StringRef name, return ContextID(known->second.first); } void APINotesWriter::addObjCProperty(ContextID contextID, StringRef name, + bool isInstance, const ObjCPropertyInfo &info) { IdentifierID nameID = Impl.getIdentifier(name); - assert(!Impl.ObjCProperties.count({contextID.Value, nameID})); - Impl.ObjCProperties[{contextID.Value, nameID}] = info; + assert(!Impl.ObjCProperties.count({contextID.Value, nameID, isInstance})); + Impl.ObjCProperties[{contextID.Value, nameID, isInstance}] = info; } void APINotesWriter::addObjCMethod(ContextID contextID, diff --git a/lib/APINotes/APINotesYAMLCompiler.cpp b/lib/APINotes/APINotesYAMLCompiler.cpp index a2188590777..b9640954f58 100644 --- a/lib/APINotes/APINotesYAMLCompiler.cpp +++ b/lib/APINotes/APINotesYAMLCompiler.cpp @@ -193,6 +193,7 @@ namespace { struct Property { StringRef Name; + llvm::Optional Kind; llvm::Optional Nullability; AvailabilityItem Availability; bool SwiftPrivate = false; @@ -348,6 +349,7 @@ namespace llvm { struct MappingTraits { static void mapping(IO &io, Property& p) { io.mapRequired("Name", p.Name); + io.mapOptional("PropertyKind", p.Kind); io.mapOptional("Nullability", p.Nullability, AbsentNullability); io.mapOptional("Availability", p.Availability.Mode); @@ -693,11 +695,20 @@ namespace { } // Write all properties. - llvm::StringSet<> knownProperties; + llvm::StringSet<> knownInstanceProperties; + llvm::StringSet<> knownClassProperties; for (const auto &prop : cl.Properties) { // Check for duplicate property definitions. - if (!knownProperties.insert(prop.Name).second) { - emitError("duplicate definition of property '" + cl.Name + "." + + if ((!prop.Kind || *prop.Kind == MethodKind::Instance) && + !knownInstanceProperties.insert(prop.Name).second) { + emitError("duplicate definition of instance property '" + cl.Name + + "." + prop.Name + "'"); + continue; + } + + if ((!prop.Kind || *prop.Kind == MethodKind::Class) && + !knownClassProperties.insert(prop.Name).second) { + emitError("duplicate definition of class property '" + cl.Name + "." + prop.Name + "'"); continue; } @@ -711,7 +722,14 @@ namespace { pInfo.SwiftName = prop.SwiftName; if (prop.Nullability) pInfo.setNullabilityAudited(*prop.Nullability); - Writer->addObjCProperty(clID, prop.Name, pInfo); + if (prop.Kind) { + Writer->addObjCProperty(clID, prop.Name, + *prop.Kind == MethodKind::Instance, pInfo); + } else { + // Add both instance and class properties with this name. + Writer->addObjCProperty(clID, prop.Name, true, pInfo); + Writer->addObjCProperty(clID, prop.Name, false, pInfo); + } } } @@ -1037,9 +1055,11 @@ namespace { } virtual void visitObjCProperty(ContextID contextID, StringRef name, + bool isInstance, const ObjCPropertyInfo &info) { Property property; property.Name = name; + property.Kind = isInstance ? MethodKind::Instance : MethodKind::Class; handleCommon(property, info); // FIXME: No way to represent "not audited for nullability". @@ -1109,6 +1129,11 @@ namespace { }; } +/// Produce a flattened, numeric value for optional method/property kinds. +static unsigned flattenPropertyKind(llvm::Optional kind) { + return kind ? (*kind == MethodKind::Instance ? 2 : 1) : 0; +} + bool api_notes::decompileAPINotes(std::unique_ptr input, llvm::raw_ostream &os) { // Try to read the file. @@ -1150,7 +1175,10 @@ bool api_notes::decompileAPINotes(std::unique_ptr input, // Sort properties. std::sort(record.Properties.begin(), record.Properties.end(), [](const Property &lhs, const Property &rhs) -> bool { - return lhs.Name < rhs.Name; + return lhs.Name < rhs.Name || + (lhs.Name == rhs.Name && + flattenPropertyKind(lhs.Kind) < + flattenPropertyKind(rhs.Kind)); }); // Sort methods. diff --git a/lib/Sema/SemaAPINotes.cpp b/lib/Sema/SemaAPINotes.cpp index 445a52c8ed1..a0c5ca88236 100644 --- a/lib/Sema/SemaAPINotes.cpp +++ b/lib/Sema/SemaAPINotes.cpp @@ -491,8 +491,12 @@ void Sema::ProcessAPINotes(Decl *D) { if (api_notes::APINotesReader *Reader = APINotes.findAPINotes(D->getLocation())) { if (auto Context = GetContext(Reader)) { + bool isInstanceProperty = + (Property->getPropertyAttributesAsWritten() & + ObjCPropertyDecl::OBJC_PR_class) == 0; if (auto Info = Reader->lookupObjCProperty(*Context, - Property->getName())) { + Property->getName(), + isInstanceProperty)) { ::ProcessAPINotes(*this, Property, *Info); } } diff --git a/lib/Sema/SemaObjCProperty.cpp b/lib/Sema/SemaObjCProperty.cpp index 5e38751f44a..a03b320a3d3 100644 --- a/lib/Sema/SemaObjCProperty.cpp +++ b/lib/Sema/SemaObjCProperty.cpp @@ -636,8 +636,6 @@ ObjCPropertyDecl *Sema::CreatePropertyDecl(Scope *S, PDecl->setInvalidDecl(); } - ProcessDeclAttributes(S, PDecl, FD.D); - // Regardless of setter/getter attribute, we save the default getter/setter // selector names in anticipation of declaration of setter/getter methods. PDecl->setGetterName(GetterSel); @@ -645,6 +643,8 @@ ObjCPropertyDecl *Sema::CreatePropertyDecl(Scope *S, PDecl->setPropertyAttributesAsWritten( makePropertyAttributesAsWritten(AttributesAsWritten)); + ProcessDeclAttributes(S, PDecl, FD.D); + if (Attributes & ObjCDeclSpec::DQ_PR_readonly) PDecl->setPropertyAttributes(ObjCPropertyDecl::OBJC_PR_readonly); diff --git a/test/APINotes/Inputs/APINotes/SomeKit.apinotes b/test/APINotes/Inputs/APINotes/SomeKit.apinotes index d251491ed43..52336df400b 100644 --- a/test/APINotes/Inputs/APINotes/SomeKit.apinotes +++ b/test/APINotes/Inputs/APINotes/SomeKit.apinotes @@ -20,6 +20,14 @@ Classes: AvailabilityMsg: "wouldn't work anyway" - Name: internalProperty Nullability: N + - Name: nonnullAInstance + PropertyKind: Instance + Nullability: N + - Name: nonnullAClass + PropertyKind: Class + Nullability: N + - Name: nonnullABoth + Nullability: N - Name: B Availability: none AvailabilityMsg: "just don't" diff --git a/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes b/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes index a585ca5f4df..ade66a10a93 100644 --- a/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes +++ b/test/APINotes/Inputs/Frameworks/SomeKit.framework/APINotes/SomeKit.apinotes @@ -12,8 +12,17 @@ Classes: Nullability: [ N, S ] Properties: - Name: intValue + PropertyKind: Instance Availability: none AvailabilityMsg: "wouldn't work anyway" + - Name: nonnullAInstance + PropertyKind: Instance + Nullability: N + - Name: nonnullAClass + PropertyKind: Class + Nullability: N + - Name: nonnullABoth + Nullability: N - Name: B Availability: none AvailabilityMsg: "just don't" diff --git a/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h b/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h index 5e0d7e0b7ab..eb25cc0c7fc 100644 --- a/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h +++ b/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h @@ -24,4 +24,15 @@ __attribute__((objc_root_class)) +(instancetype)processInfo; @end +@interface A(NonNullProperties) +@property (nonatomic, readwrite, retain) A *nonnullAInstance; +@property (class, nonatomic, readwrite, retain) A *nonnullAInstance; + +@property (nonatomic, readwrite, retain) A *nonnullAClass; +@property (class, nonatomic, readwrite, retain) A *nonnullAClass; + +@property (nonatomic, readwrite, retain) A *nonnullABoth; +@property (class, nonatomic, readwrite, retain) A *nonnullABoth; +@end + #endif diff --git a/test/APINotes/Inputs/roundtrip.apinotes b/test/APINotes/Inputs/roundtrip.apinotes index 09027347901..10ac249817d 100644 --- a/test/APINotes/Inputs/roundtrip.apinotes +++ b/test/APINotes/Inputs/roundtrip.apinotes @@ -90,12 +90,14 @@ Classes: SwiftName: 'beginDragginSession(_:event:source:)' Properties: - Name: enclosingScrollView + PropertyKind: Instance Nullability: O Availability: available AvailabilityMsg: '' SwiftPrivate: false SwiftName: enclosing - Name: makeBackingLayer + PropertyKind: Class Nullability: N Availability: available AvailabilityMsg: '' diff --git a/test/APINotes/nullability.m b/test/APINotes/nullability.m index ce4beabb597..486b2c5f366 100644 --- a/test/APINotes/nullability.m +++ b/test/APINotes/nullability.m @@ -3,12 +3,20 @@ #import - int main() { A *a; [a transform: 0 integer: 0]; // expected-warning{{null passed to a callee that requires a non-null argument}} + [a setNonnullAInstance: 0]; // expected-warning{{null passed to a callee that requires a non-null argument}} + [A setNonnullAInstance: 0]; // no warning + + [a setNonnullAClass: 0]; // no warning + [A setNonnullAClass: 0]; // expected-warning{{null passed to a callee that requires a non-null argument}} + + [a setNonnullABoth: 0]; // expected-warning{{null passed to a callee that requires a non-null argument}} + [A setNonnullABoth: 0]; // expected-warning{{null passed to a callee that requires a non-null argument}} + return 0; } diff --git a/test/APINotes/yaml-reader-errors.c b/test/APINotes/yaml-reader-errors.c index 51dfe3aed84..3e01eaaef03 100644 --- a/test/APINotes/yaml-reader-errors.c +++ b/test/APINotes/yaml-reader-errors.c @@ -34,7 +34,7 @@ AvailabilityMsg: iOSOnly Nullability: N Availability: iOS AvailabilityMsg: iOSOnly -# CHECK: duplicate definition of property 'UIFont.familyName' +# CHECK: duplicate definition of instance property 'UIFont.familyName' - Name: familyName Nullability: N Availability: iOS