[PATCH] D91104: APINotes: add property models for YAML attributes

Gabor Marton via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 19 06:34:32 PST 2020


martong added inline comments.


================
Comment at: clang/include/clang/APINotes/Types.h:60
+  /// Whether SwiftPrivate was specified.
+  unsigned SwiftPrivateSpecified : 1;
+
----------------
I was wondering whether Swift specific properties are meaningful to all descendants of `CommonEntityInfo`. With other words, can we attach the swift private attribute to all kind of declarations? If not, perhaps then we need a "middle" base class injected into the class hierarchy.

And I have the same question for `CommonTypeInfo`, regarding SwiftBridge and other Swift sweeties.


================
Comment at: clang/include/clang/APINotes/Types.h:125
+class CommonTypeInfo : public CommonEntityInfo {
+  /// The Swift type to which a given type is bridged.
+  ///
----------------
This comment is probably off.


================
Comment at: clang/include/clang/APINotes/Types.h:529
+  /// Adds the return type info.
+  void addReturnTypeInfo(NullabilityKind kind) { addTypeInfo(0, kind); }
+
----------------
Do we document somewhere that the `0` index means the return type? Maybe adding a constant like `ReturnIdx` could resolve this magic number.


================
Comment at: clang/include/clang/APINotes/Types.h:531
+
+  /// Adds the return type info.
+  void addParamTypeInfo(unsigned index, NullabilityKind kind) {
----------------
Probably this comment is off.


================
Comment at: clang/lib/APINotes/APINotesTypes.cpp:20
+
+void ObjCContextInfo::dump(llvm::raw_ostream &OS) {
+  OS << HasDefaultNullability << ' ' << DefaultNullability << ' '
----------------
Perhaps it would be worth to add `dump` methods for the other `Info` classes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D91104/new/

https://reviews.llvm.org/D91104



More information about the cfe-commits mailing list