compnerd marked 2 inline comments as done.
compnerd added inline comments.
================
Comment at: clang/include/clang/APINotes/Types.h:60
+ /// Whether SwiftPrivate was specified.
+ unsigned SwiftPrivateSpecified : 1;
+
----------------
martong wrote:
> 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.
The `__swift_private__` attribute applies to all declarations, so I think that
it makes sense as is. I suppose that we can do multiple inheritance if you
want to separate the Swift and ObjC attributes.
The `__swift_bridge__` attribute only applies to tags, typedefs, ObjC
interfaces and protocols. The `__swift_name__` attribute applies to all
declarations.
The `__ns_error_domain__` attribute is ObjC specific, though Swift does care
about it. It only applies to enum types though.
The vast majority of the Swift attributes do apply to a wide variety of types,
and I think that the current structure makes sense. It seems a bit unnecessary
to split them out further into a structure and loose the packing, but I suppose
that one option would be to add wrapper structs around all the language
specific options, but that comes at the cost of higher memory usage for clang,
which is already significant at times.
================
Comment at: clang/include/clang/APINotes/Types.h:125
+class CommonTypeInfo : public CommonEntityInfo {
+ /// The Swift type to which a given type is bridged.
+ ///
----------------
martong wrote:
> This comment is probably off.
I think that this comment is correct, it is describing the `SwiftBridge` field.
================
Comment at: clang/include/clang/APINotes/Types.h:529
+ /// Adds the return type info.
+ void addReturnTypeInfo(NullabilityKind kind) { addTypeInfo(0, kind); }
+
----------------
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?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91104/new/
https://reviews.llvm.org/D91104
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
