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

Saleem Abdulrasool via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 19 11:16:02 PST 2020


compnerd added inline comments.


================
Comment at: clang/include/clang/APINotes/Types.h:529
+  /// Adds the return type info.
+  void addReturnTypeInfo(NullabilityKind kind) { addTypeInfo(0, kind); }
+
----------------
compnerd wrote:
> martong wrote:
> > Do we document somewhere that the `0` index means the return type? Maybe adding a constant like `ReturnIdx` could resolve this magic number.
> Sure, I can add a constant for that.  I don't think that we have that documented, but, I think that the constant should be sufficient for that no?
Rather, we don't have it commented outside of the header (we document it in the nullability payload internal docs above ~491).


================
Comment at: clang/lib/APINotes/APINotesTypes.cpp:20
+
+void ObjCContextInfo::dump(llvm::raw_ostream &OS) {
+  OS << HasDefaultNullability << ' ' << DefaultNullability << ' '
----------------
martong wrote:
> Perhaps it would be worth to add `dump` methods for the other `Info` classes.
Well, I don't think it really hurts, the dump methods should get dropped in released toolchains, so this is just painful to write the dumps is all.  Added.


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