[APINotes] Upstream APINotes YAML compiler #71413
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This value actually represents a number of arguments the selector takes, not the number of identifiers. These will be different if the selector takes no arguments.
|
@llvm/pr-subscribers-clang Author: Egor Zhdan (egorzhdan) ChangesThis upstreams more of the Clang API Notes functionality that is currently implemented in the Apple fork: https://github.com/apple/llvm-project/tree/next/clang/lib/APINotes Patch is 23.21 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/71413.diff 6 Files Affected:
diff --git a/clang/include/clang/APINotes/APINotesYAMLCompiler.h b/clang/include/clang/APINotes/APINotesYAMLCompiler.h
index 6098d0ee36fc477..9c24ed85b6a124a 100644
--- a/clang/include/clang/APINotes/APINotesYAMLCompiler.h
+++ b/clang/include/clang/APINotes/APINotesYAMLCompiler.h
@@ -10,14 +10,25 @@
#define LLVM_CLANG_APINOTES_APINOTESYAMLCOMPILER_H
#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/SourceMgr.h"
#include "llvm/Support/raw_ostream.h"
+namespace clang {
+class FileEntry;
+} // namespace clang
+
namespace clang {
namespace api_notes {
/// Parses the APINotes YAML content and writes the representation back to the
/// specified stream. This provides a means of testing the YAML processing of
/// the APINotes format.
bool parseAndDumpAPINotes(llvm::StringRef YI, llvm::raw_ostream &OS);
+
+/// Converts API notes from YAML format to binary format.
+bool compileAPINotes(llvm::StringRef YAMLInput, const FileEntry *SourceFile,
+ llvm::raw_ostream &OS,
+ llvm::SourceMgr::DiagHandlerTy DiagHandler = nullptr,
+ void *DiagHandlerCtxt = nullptr);
} // namespace api_notes
} // namespace clang
diff --git a/clang/include/clang/APINotes/Types.h b/clang/include/clang/APINotes/Types.h
index b74244bc8f1cbd3..79595abcf7d02d9 100644
--- a/clang/include/clang/APINotes/Types.h
+++ b/clang/include/clang/APINotes/Types.h
@@ -766,6 +766,7 @@ struct Context {
/// data they contain; it is up to the user to ensure that the data
/// referenced by the identifier list persists.
struct ObjCSelectorRef {
+ unsigned NumArgs;
llvm::ArrayRef<llvm::StringRef> Identifiers;
};
} // namespace api_notes
diff --git a/clang/lib/APINotes/APINotesFormat.h b/clang/lib/APINotes/APINotesFormat.h
index 5897b45d3796d0e..615314c46f09cac 100644
--- a/clang/lib/APINotes/APINotesFormat.h
+++ b/clang/lib/APINotes/APINotesFormat.h
@@ -247,7 +247,7 @@ using EnumConstantDataLayout =
/// A stored Objective-C selector.
struct StoredObjCSelector {
- unsigned NumPieces;
+ unsigned NumArgs;
llvm::SmallVector<IdentifierID, 2> Identifiers;
};
@@ -302,7 +302,7 @@ template <> struct DenseMapInfo<clang::api_notes::StoredObjCSelector> {
static unsigned
getHashValue(const clang::api_notes::StoredObjCSelector &Selector) {
- auto hash = llvm::hash_value(Selector.NumPieces);
+ auto hash = llvm::hash_value(Selector.NumArgs);
hash = hash_combine(hash, Selector.Identifiers.size());
for (auto piece : Selector.Identifiers)
hash = hash_combine(hash, static_cast<unsigned>(piece));
@@ -313,7 +313,7 @@ template <> struct DenseMapInfo<clang::api_notes::StoredObjCSelector> {
static bool isEqual(const clang::api_notes::StoredObjCSelector &LHS,
const clang::api_notes::StoredObjCSelector &RHS) {
- return LHS.NumPieces == RHS.NumPieces && LHS.Identifiers == RHS.Identifiers;
+ return LHS.NumArgs == RHS.NumArgs && LHS.Identifiers == RHS.Identifiers;
}
};
diff --git a/clang/lib/APINotes/APINotesReader.cpp b/clang/lib/APINotes/APINotesReader.cpp
index 2cbf5fd3bf50301..ff9b95d9bf75e3d 100644
--- a/clang/lib/APINotes/APINotesReader.cpp
+++ b/clang/lib/APINotes/APINotesReader.cpp
@@ -421,7 +421,7 @@ class ObjCSelectorTableInfo {
static internal_key_type ReadKey(const uint8_t *Data, unsigned Length) {
internal_key_type Key;
- Key.NumPieces =
+ Key.NumArgs =
endian::readNext<uint16_t, llvm::endianness::little, unaligned>(Data);
unsigned NumIdents = (Length - sizeof(uint16_t)) / sizeof(uint32_t);
for (unsigned i = 0; i != NumIdents; ++i) {
@@ -741,6 +741,7 @@ APINotesReader::Implementation::getSelector(ObjCSelectorRef Selector) {
// Translate the identifiers.
StoredObjCSelector Key;
+ Key.NumArgs = Selector.NumArgs;
for (auto Ident : Selector.Identifiers) {
if (auto IdentID = getIdentifier(Ident)) {
Key.Identifiers.push_back(*IdentID);
diff --git a/clang/lib/APINotes/APINotesWriter.cpp b/clang/lib/APINotes/APINotesWriter.cpp
index 770d78e22050c01..62a2ab1799913a1 100644
--- a/clang/lib/APINotes/APINotesWriter.cpp
+++ b/clang/lib/APINotes/APINotesWriter.cpp
@@ -823,7 +823,7 @@ class ObjCSelectorTableInfo {
void EmitKey(raw_ostream &OS, key_type_ref Key, unsigned) {
llvm::support::endian::Writer writer(OS, llvm::endianness::little);
- writer.write<uint16_t>(Key.NumPieces);
+ writer.write<uint16_t>(Key.NumArgs);
for (auto Identifier : Key.Identifiers)
writer.write<uint32_t>(Identifier);
}
diff --git a/clang/lib/APINotes/APINotesYAMLCompiler.cpp b/clang/lib/APINotes/APINotesYAMLCompiler.cpp
index 647455111214c59..7e43cc981f35968 100644
--- a/clang/lib/APINotes/APINotesYAMLCompiler.cpp
+++ b/clang/lib/APINotes/APINotesYAMLCompiler.cpp
@@ -14,14 +14,17 @@
//
#include "clang/APINotes/APINotesYAMLCompiler.h"
+#include "clang/APINotes/APINotesWriter.h"
#include "clang/APINotes/Types.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/Specifiers.h"
+#include "llvm/ADT/StringSet.h"
+#include "llvm/Support/SourceMgr.h"
#include "llvm/Support/VersionTuple.h"
-#include "llvm/Support/YAMLParser.h"
#include "llvm/Support/YAMLTraits.h"
#include <optional>
#include <vector>
+
using namespace clang;
using namespace api_notes;
@@ -635,3 +638,496 @@ bool clang::api_notes::parseAndDumpAPINotes(StringRef YI,
return false;
}
+
+namespace {
+using namespace api_notes;
+
+class YAMLConverter {
+ const Module &TheModule;
+ APINotesWriter Writer;
+ llvm::raw_ostream &OS;
+ llvm::SourceMgr::DiagHandlerTy DiagHandler;
+ void *DiagHandlerCtxt;
+ bool ErrorOccured;
+
+ /// Emit a diagnostic
+ bool emitError(llvm::Twine Message) {
+ DiagHandler(
+ llvm::SMDiagnostic("", llvm::SourceMgr::DK_Error, Message.str()),
+ DiagHandlerCtxt);
+ ErrorOccured = true;
+ return true;
+ }
+
+public:
+ YAMLConverter(const Module &TheModule, const FileEntry *SourceFile,
+ llvm::raw_ostream &OS,
+ llvm::SourceMgr::DiagHandlerTy DiagHandler,
+ void *DiagHandlerCtxt)
+ : TheModule(TheModule), Writer(TheModule.Name, SourceFile), OS(OS),
+ DiagHandler(DiagHandler), DiagHandlerCtxt(DiagHandlerCtxt),
+ ErrorOccured(false) {}
+
+ bool convertAvailability(const AvailabilityItem &In,
+ CommonEntityInfo &OutInfo, llvm::StringRef APIName) {
+ // Populate the unavailability information.
+ OutInfo.Unavailable = (In.Mode == APIAvailability::None);
+ OutInfo.UnavailableInSwift = (In.Mode == APIAvailability::NonSwift);
+ if (OutInfo.Unavailable || OutInfo.UnavailableInSwift) {
+ OutInfo.UnavailableMsg = std::string(In.Msg);
+ } else {
+ if (!In.Msg.empty()) {
+ emitError("availability message for available API '" + APIName +
+ "' will not be used");
+ }
+ }
+ return false;
+ }
+
+ void convertParams(const ParamsSeq &Params, FunctionInfo &OutInfo) {
+ for (const auto &P : Params) {
+ ParamInfo PI;
+ if (P.Nullability)
+ PI.setNullabilityAudited(*P.Nullability);
+ PI.setNoEscape(P.NoEscape);
+ PI.setType(std::string(P.Type));
+ PI.setRetainCountConvention(P.RetainCountConvention);
+ while (OutInfo.Params.size() <= P.Position) {
+ OutInfo.Params.push_back(ParamInfo());
+ }
+ OutInfo.Params[P.Position] |= PI;
+ }
+ }
+
+ void convertNullability(const NullabilitySeq &Nullability,
+ std::optional<NullabilityKind> NullabilityOfRet,
+ FunctionInfo &OutInfo, llvm::StringRef APIName) {
+ if (Nullability.size() > FunctionInfo::getMaxNullabilityIndex()) {
+ emitError("nullability info for " + APIName + " does not fit");
+ return;
+ }
+
+ bool audited = false;
+ unsigned int idx = 1;
+ for (auto i = Nullability.begin(), e = Nullability.end(); i != e;
+ ++i, ++idx) {
+ OutInfo.addTypeInfo(idx, *i);
+ audited = true;
+ }
+ if (NullabilityOfRet) {
+ OutInfo.addTypeInfo(0, *NullabilityOfRet);
+ audited = true;
+ } else if (audited) {
+ OutInfo.addTypeInfo(0, NullabilityKind::NonNull);
+ }
+ if (audited) {
+ OutInfo.NullabilityAudited = audited;
+ OutInfo.NumAdjustedNullable = idx;
+ }
+ }
+
+ /// Convert the common parts of an entity from YAML.
+ template <typename T>
+ bool convertCommon(const T &Common, CommonEntityInfo &Info,
+ StringRef APIName) {
+ convertAvailability(Common.Availability, Info, APIName);
+ Info.setSwiftPrivate(Common.SwiftPrivate);
+ Info.SwiftName = std::string(Common.SwiftName);
+ return false;
+ }
+
+ /// Convert the common parts of a type entity from YAML.
+ template <typename T>
+ bool convertCommonType(const T &Common, CommonTypeInfo &Info,
+ StringRef APIName) {
+ if (convertCommon(Common, Info, APIName))
+ return true;
+
+ if (Common.SwiftBridge)
+ Info.setSwiftBridge(std::string(*Common.SwiftBridge));
+ Info.setNSErrorDomain(Common.NSErrorDomain);
+ return false;
+ }
+
+ // Translate from Method into ObjCMethodInfo and write it out.
+ void convertMethod(const Method &TheMethod, ContextID ClassID,
+ StringRef ClassName, VersionTuple SwiftVersion) {
+ ObjCMethodInfo MInfo;
+
+ if (convertCommon(TheMethod, MInfo, TheMethod.Selector))
+ return;
+
+ // Check if the selector ends with ':' to determine if it takes arguments.
+ bool takesArguments = TheMethod.Selector.endswith(":");
+
+ // Split the selector into pieces.
+ llvm::SmallVector<StringRef, 4> a;
+ TheMethod.Selector.split(a, ":", /*MaxSplit*/ -1, /*KeepEmpty*/ false);
+ if (!takesArguments && a.size() > 1) {
+ emitError("selector " + TheMethod.Selector +
+ "is missing a ':' at the end");
+ return;
+ }
+
+ // Construct ObjCSelectorRef.
+ api_notes::ObjCSelectorRef selectorRef;
+ selectorRef.NumArgs = !takesArguments ? 0 : a.size();
+ selectorRef.Identifiers = a;
+
+ // Translate the initializer info.
+ MInfo.DesignatedInit = TheMethod.DesignatedInit;
+ MInfo.RequiredInit = TheMethod.Required;
+ if (TheMethod.FactoryAsInit != FactoryAsInitKind::Infer) {
+ emitError("'FactoryAsInit' is no longer valid; "
+ "use 'SwiftName' instead");
+ }
+ MInfo.ResultType = std::string(TheMethod.ResultType);
+
+ // Translate parameter information.
+ convertParams(TheMethod.Params, MInfo);
+
+ // Translate nullability info.
+ convertNullability(TheMethod.Nullability, TheMethod.NullabilityOfRet, MInfo,
+ TheMethod.Selector);
+
+ MInfo.setRetainCountConvention(TheMethod.RetainCountConvention);
+
+ // Write it.
+ Writer.addObjCMethod(ClassID, selectorRef,
+ TheMethod.Kind == MethodKind::Instance, MInfo,
+ SwiftVersion);
+ }
+
+ void convertContext(std::optional<ContextID> ParentContextID,
+ const Class &TheClass, ContextKind Kind,
+ VersionTuple SwiftVersion) {
+ // Write the class.
+ ObjCContextInfo CInfo;
+
+ if (convertCommonType(TheClass, CInfo, TheClass.Name))
+ return;
+
+ if (TheClass.AuditedForNullability)
+ CInfo.setDefaultNullability(NullabilityKind::NonNull);
+ if (TheClass.SwiftImportAsNonGeneric)
+ CInfo.setSwiftImportAsNonGeneric(*TheClass.SwiftImportAsNonGeneric);
+ if (TheClass.SwiftObjCMembers)
+ CInfo.setSwiftObjCMembers(*TheClass.SwiftObjCMembers);
+
+ ContextID CtxID = Writer.addObjCContext(ParentContextID, TheClass.Name,
+ Kind, CInfo, SwiftVersion);
+
+ // Write all methods.
+ llvm::StringMap<std::pair<bool, bool>> KnownMethods;
+ for (const auto &method : TheClass.Methods) {
+ // Check for duplicate method definitions.
+ bool IsInstanceMethod = method.Kind == MethodKind::Instance;
+ bool &Known = IsInstanceMethod ? KnownMethods[method.Selector].first
+ : KnownMethods[method.Selector].second;
+ if (Known) {
+ emitError(llvm::Twine("duplicate definition of method '") +
+ (IsInstanceMethod ? "-" : "+") + "[" + TheClass.Name + " " +
+ method.Selector + "]'");
+ continue;
+ }
+ Known = true;
+
+ convertMethod(method, CtxID, TheClass.Name, SwiftVersion);
+ }
+
+ // Write all properties.
+ llvm::StringSet<> KnownInstanceProperties;
+ llvm::StringSet<> KnownClassProperties;
+ for (const auto &Property : TheClass.Properties) {
+ // Check for duplicate property definitions.
+ if ((!Property.Kind || *Property.Kind == MethodKind::Instance) &&
+ !KnownInstanceProperties.insert(Property.Name).second) {
+ emitError("duplicate definition of instance property '" +
+ TheClass.Name + "." + Property.Name + "'");
+ continue;
+ }
+
+ if ((!Property.Kind || *Property.Kind == MethodKind::Class) &&
+ !KnownClassProperties.insert(Property.Name).second) {
+ emitError("duplicate definition of class property '" + TheClass.Name +
+ "." + Property.Name + "'");
+ continue;
+ }
+
+ // Translate from Property into ObjCPropertyInfo.
+ ObjCPropertyInfo PInfo;
+ convertAvailability(Property.Availability, PInfo, Property.Name);
+ PInfo.setSwiftPrivate(Property.SwiftPrivate);
+ PInfo.SwiftName = std::string(Property.SwiftName);
+ if (Property.Nullability)
+ PInfo.setNullabilityAudited(*Property.Nullability);
+ if (Property.SwiftImportAsAccessors)
+ PInfo.setSwiftImportAsAccessors(*Property.SwiftImportAsAccessors);
+ PInfo.setType(std::string(Property.Type));
+ if (Property.Kind) {
+ Writer.addObjCProperty(CtxID, Property.Name,
+ *Property.Kind == MethodKind::Instance, PInfo,
+ SwiftVersion);
+ } else {
+ // Add both instance and class properties with this name.
+ Writer.addObjCProperty(CtxID, Property.Name, true, PInfo, SwiftVersion);
+ Writer.addObjCProperty(CtxID, Property.Name, false, PInfo,
+ SwiftVersion);
+ }
+ }
+ }
+
+ void convertNamespaceContext(std::optional<ContextID> ParentContextID,
+ const Namespace &TheNamespace,
+ VersionTuple SwiftVersion) {
+ // Write the namespace.
+ ObjCContextInfo CInfo;
+
+ if (convertCommon(TheNamespace, CInfo, TheNamespace.Name))
+ return;
+
+ ContextID CtxID =
+ Writer.addObjCContext(ParentContextID, TheNamespace.Name,
+ ContextKind::Namespace, CInfo, SwiftVersion);
+
+ convertTopLevelItems(Context(CtxID, ContextKind::Namespace),
+ TheNamespace.Items, SwiftVersion);
+ }
+
+ void convertTopLevelItems(std::optional<Context> Ctx,
+ const TopLevelItems &TLItems,
+ VersionTuple SwiftVersion) {
+ std::optional<ContextID> CtxID =
+ Ctx ? std::optional(Ctx->id) : std::nullopt;
+
+ // Write all classes.
+ llvm::StringSet<> KnownClasses;
+ for (const auto &Class : TLItems.Classes) {
+ // Check for duplicate class definitions.
+ if (!KnownClasses.insert(Class.Name).second) {
+ emitError("multiple definitions of class '" + Class.Name + "'");
+ continue;
+ }
+
+ convertContext(CtxID, Class, ContextKind::ObjCClass, SwiftVersion);
+ }
+
+ // Write all protocols.
+ llvm::StringSet<> KnownProtocols;
+ for (const auto &Protocol : TLItems.Protocols) {
+ // Check for duplicate protocol definitions.
+ if (!KnownProtocols.insert(Protocol.Name).second) {
+ emitError("multiple definitions of protocol '" + Protocol.Name + "'");
+ continue;
+ }
+
+ convertContext(CtxID, Protocol, ContextKind::ObjCProtocol, SwiftVersion);
+ }
+
+ // Write all namespaces.
+ llvm::StringSet<> KnownNamespaces;
+ for (const auto &Namespace : TLItems.Namespaces) {
+ // Check for duplicate namespace definitions.
+ if (!KnownNamespaces.insert(Namespace.Name).second) {
+ emitError("multiple definitions of namespace '" + Namespace.Name + "'");
+ continue;
+ }
+
+ convertNamespaceContext(CtxID, Namespace, SwiftVersion);
+ }
+
+ // Write all global variables.
+ llvm::StringSet<> KnownGlobals;
+ for (const auto &Global : TLItems.Globals) {
+ // Check for duplicate global variables.
+ if (!KnownGlobals.insert(Global.Name).second) {
+ emitError("multiple definitions of global variable '" + Global.Name +
+ "'");
+ continue;
+ }
+
+ GlobalVariableInfo GVInfo;
+ convertAvailability(Global.Availability, GVInfo, Global.Name);
+ GVInfo.setSwiftPrivate(Global.SwiftPrivate);
+ GVInfo.SwiftName = std::string(Global.SwiftName);
+ if (Global.Nullability)
+ GVInfo.setNullabilityAudited(*Global.Nullability);
+ GVInfo.setType(std::string(Global.Type));
+ Writer.addGlobalVariable(Ctx, Global.Name, GVInfo, SwiftVersion);
+ }
+
+ // Write all global functions.
+ llvm::StringSet<> KnownFunctions;
+ for (const auto &Function : TLItems.Functions) {
+ // Check for duplicate global functions.
+ if (!KnownFunctions.insert(Function.Name).second) {
+ emitError("multiple definitions of global function '" + Function.Name +
+ "'");
+ continue;
+ }
+
+ GlobalFunctionInfo GFInfo;
+ convertAvailability(Function.Availability, GFInfo, Function.Name);
+ GFInfo.setSwiftPrivate(Function.SwiftPrivate);
+ GFInfo.SwiftName = std::string(Function.SwiftName);
+ convertParams(Function.Params, GFInfo);
+ convertNullability(Function.Nullability, Function.NullabilityOfRet,
+ GFInfo, Function.Name);
+ GFInfo.ResultType = std::string(Function.ResultType);
+ GFInfo.setRetainCountConvention(Function.RetainCountConvention);
+ Writer.addGlobalFunction(Ctx, Function.Name, GFInfo, SwiftVersion);
+ }
+
+ // Write all enumerators.
+ llvm::StringSet<> KnownEnumConstants;
+ for (const auto &EnumConstant : TLItems.EnumConstants) {
+ // Check for duplicate enumerators
+ if (!KnownEnumConstants.insert(EnumConstant.Name).second) {
+ emitError("multiple definitions of enumerator '" + EnumConstant.Name +
+ "'");
+ continue;
+ }
+
+ EnumConstantInfo ECInfo;
+ convertAvailability(EnumConstant.Availability, ECInfo, EnumConstant.Name);
+ ECInfo.setSwiftPrivate(EnumConstant.SwiftPrivate);
+ ECInfo.SwiftName = std::string(EnumConstant.SwiftName);
+ Writer.addEnumConstant(EnumConstant.Name, ECInfo, SwiftVersion);
+ }
+
+ // Write all tags.
+ llvm::StringSet<> KnownTags;
+ for (const auto &Tag : TLItems.Tags) {
+ // Check for duplicate tag definitions.
+ if (!KnownTags.insert(Tag.Name).second) {
+ emitError("multiple definitions of tag '" + Tag.Name + "'");
+ continue;
+ }
+
+ TagInfo TInfo;
+ if (convertCommonType(Tag, TInfo, Tag.Name))
+ continue;
+
+ if ((Tag.SwiftRetainOp || Tag.SwiftReleaseOp) && !Tag.SwiftImportAs) {
+ emitError(llvm::Twine("should declare SwiftImportAs to use "
+ "SwiftRetainOp and SwiftReleaseOp (for ") +
+ Tag.Name + ")");
+ continue;
+ }
+ if (Tag.SwiftReleaseOp.has_value() != Tag.SwiftRetainOp.has_value()) {
+ emitError(llvm::Twine("should declare both SwiftReleaseOp and "
+ "SwiftRetainOp (for ") +
+ Tag.Name + ")");
+ continue;
+ }
+
+ if (Tag.SwiftImportAs)
+ TInfo.SwiftImportAs = Tag.SwiftImportAs;
+ if (Tag.SwiftRetainOp)
+ TInfo.SwiftRetainOp = Tag.SwiftRetainOp;
+ if (Tag.SwiftReleaseOp)
+ TInfo.SwiftReleaseOp = Tag.SwiftReleaseOp;
+
+ if (Tag.EnumConvenienceKind) {
+ if (Tag.EnumExtensibility) {
+ emitError(
+ llvm::Twine("cannot mix EnumKind and EnumExtensibility (for ") +
+ Tag.Name + ")");
+ continue;
+ }...
[truncated]
|
compnerd
reviewed
Nov 7, 2023
| : KnownMethods[method.Selector].second; | ||
| if (Known) { | ||
| emitError(llvm::Twine("duplicate definition of method '") + | ||
| (IsInstanceMethod ? "-" : "+") + "[" + TheClass.Name + " " + |
There was a problem hiding this comment.
I wonder if it is useful to extract the string conversion for the selector, though I suppose that we couldn't take advantage of the Twine if we did that.
Comment on lines
867
to
856
| if (Property.Kind) { | ||
| Writer.addObjCProperty(CtxID, Property.Name, | ||
| *Property.Kind == MethodKind::Instance, PInfo, | ||
| SwiftVersion); | ||
| } else { | ||
| // Add both instance and class properties with this name. | ||
| Writer.addObjCProperty(CtxID, Property.Name, true, PInfo, SwiftVersion); | ||
| Writer.addObjCProperty(CtxID, Property.Name, false, PInfo, | ||
| SwiftVersion); | ||
| } |
There was a problem hiding this comment.
I wonder if it is better to write this as:
// If the property is not explicit, add both instance and class properties.
Writer.addObjCProperty(CtxID, Property.Name,
Property.Kind ? *Property.Kind == MethodKind::Instance : true,
PI, SwiftVersion);
if (!Property.Kind)
Writer.addObjCProperty(CtxID, Property.Name, false, PI, SwiftVersion);There was a problem hiding this comment.
I think the original code is slightly easier to understand, but I don't have a strong opinion
There was a problem hiding this comment.
Okay - well, then I would say that it might be nice to hoist the comment from the else-case to before the condition.
egorzhdan
force-pushed
the
upstream-apinotes-compiler
branch
3 times, most recently
from
a95d72d
to
59c1842
Compare
compnerd
reviewed
Nov 13, 2023
Comment on lines
694
to
696
| if (OutInfo.Params.size() <= P.Position) { | ||
| OutInfo.Params.resize(P.Position + 1); | ||
| } |
There was a problem hiding this comment.
Unnecessary braces
Suggested change
| if (OutInfo.Params.size() <= P.Position) { | |
| OutInfo.Params.resize(P.Position + 1); | |
| } | |
| if (OutInfo.Params.size() <= P.Position) | |
| OutInfo.Params.resize(P.Position + 1); |
Comment on lines
867
to
856
| if (Property.Kind) { | ||
| Writer.addObjCProperty(CtxID, Property.Name, | ||
| *Property.Kind == MethodKind::Instance, PInfo, | ||
| SwiftVersion); | ||
| } else { | ||
| // Add both instance and class properties with this name. | ||
| Writer.addObjCProperty(CtxID, Property.Name, true, PInfo, SwiftVersion); | ||
| Writer.addObjCProperty(CtxID, Property.Name, false, PInfo, | ||
| SwiftVersion); | ||
| } |
There was a problem hiding this comment.
Okay - well, then I would say that it might be nice to hoist the comment from the else-case to before the condition.
compnerd
approved these changes
Nov 13, 2023
Comment on lines
1075
to
1078
| for (const auto &Versioned : M.SwiftVersions) { | ||
| convertTopLevelItems(/* context */ std::nullopt, Versioned.Items, | ||
| Versioned.Version); | ||
| } |
There was a problem hiding this comment.
Suggested change
| for (const auto &Versioned : M.SwiftVersions) { | |
| convertTopLevelItems(/* context */ std::nullopt, Versioned.Items, | |
| Versioned.Version); | |
| } | |
| for (const auto &Versioned : M.SwiftVersions) | |
| convertTopLevelItems(/* context */ std::nullopt, Versioned.Items, | |
| Versioned.Version); |
Comment on lines
679
to
682
| if (!Availability.Msg.empty()) { | ||
| emitError(llvm::Twine("availability message for available API '") + | ||
| APIName + "' will not be used"); | ||
| } |
There was a problem hiding this comment.
Suggested change
| if (!Availability.Msg.empty()) { | |
| emitError(llvm::Twine("availability message for available API '") + | |
| APIName + "' will not be used"); | |
| } | |
| if (!Availability.Msg.empty()) | |
| emitError(llvm::Twine("availability message for available API '") + | |
| APIName + "' will not be used"); |
This upstreams more of the Clang API Notes functionality that is currently implemented in the Apple fork: https://github.com/apple/llvm-project/tree/next/clang/lib/APINotes
egorzhdan
force-pushed
the
upstream-apinotes-compiler
branch
from
59c1842
to
2516116
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This upstreams more of the Clang API Notes functionality that is currently implemented in the Apple fork: https://github.com/apple/llvm-project/tree/next/clang/lib/APINotes