compnerd marked 10 inline comments as done.
compnerd added inline comments.
================
Comment at: clang/include/clang/Basic/Attr.td:2122
+def SwiftError : InheritableAttr {
+ let Spellings = [GCC<"swift_error">];
+ let Args = [
----------------
aaron.ballman wrote:
> Is this attribute also supported by GCC? It looked like it wasn't from my
> quick check, so I'd recommend switching the spelling to be `Clang` instead.
> Note, this will give you `[[clang::swift_error(...)]]` in both C and C++ as
> well. If you only want the GNU spelling, use `GNU`.
Ah, oops, this should be `GNU`.
================
Comment at: clang/include/clang/Basic/Attr.td:2128
+ ];
+ let Subjects = SubjectList<[Function, ObjCMethod], ErrorDiag>;
+ let Documentation = [SwiftErrorDocs];
----------------
aaron.ballman wrote:
> Should this apply to function-like things such as blocks or function
> pointers? If so, you should use `HasFunctionProto`.
I believe not.
================
Comment at: clang/include/clang/Basic/AttrDocs.td:3394
+parameter. Currently, the error parameter is always the last parameter of type
+``NSError**`` or ``CFErrorRef*``. Swift will remove the error parameter from
+the imported API. When calling the API, Swift will always pass a valid address
----------------
aaron.ballman wrote:
> Canonical type or are typedefs to these types also fine? May want to mention
> that the type has to be cv-unqualified, but I do wonder whether something
> like `NSError * const *` is fine.
I am definitely not the authority on this, but I believe that the common thing
is to take `NSError **` or `CFErrorRef **` by canonical name. The cv-qualified
versions, except on the outermost pointer, is something that can be treated as
valid, though it is certainly extremely unusual. I've added test cases for
them as well.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87331/new/
https://reviews.llvm.org/D87331
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
