[PATCH] D88446: docs: add documentation describing API Notes

2020-10-05 Thread Saleem Abdulrasool via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG85d506400081: docs: add documentation describing API Notes 
(authored by compnerd).

Changed prior to commit:
  https://reviews.llvm.org/D88446?vs=295924=296243#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

Files:
  clang/docs/APINotes.rst
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
  clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes

Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
@@ -0,0 +1,15 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector: "privateTransform:input:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+Properties:
+  - Name: internalProperty
+Nullability: N
+Protocols:
+  - Name: InternalProtocol
+Availability: none
+AvailabilityMsg: "not for you"
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
@@ -0,0 +1,4 @@
+@interface A(ExplicitNullabilityProperties)
+@property (nonatomic, readwrite, retain, nonnull) A *explicitNonnullInstance;
+@property (nonatomic, readwrite, retain, nullable) A *explicitNullableInstance;
+@end
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
@@ -0,0 +1,60 @@
+#ifndef SOMEKIT_H
+#define SOMEKIT_H
+
+__attribute__((objc_root_class))
+@interface A
+-(A*)transform:(A*)input;
+-(A*)transform:(A*)input integer:(int)integer;
+
+@property (nonatomic, readonly, retain) A* someA;
+@property (nonatomic, retain) A* someOtherA;
+
+@property (nonatomic) int intValue;
+@end
+
+@interface B : A
+@end
+
+@interface C : A
+- (instancetype)init;
+- (instancetype)initWithA:(A*)a;
+@end
+
+@interface ProcessInfo : A
++(instancetype)processInfo;
+@end
+
+@interface A(NonNullProperties)
+@property (nonatomic, readwrite, retain) A *nonnullAInstance;
+@property (class, nonatomic, readwrite, retain) A *nonnullAInstance;
+
+@property (nonatomic, readwrite, retain) A *nonnullAClass;
+@property (class, nonatomic, readwrite, retain) A *nonnullAClass;
+
+@property (nonatomic, readwrite, retain) A *nonnullABoth;
+@property (class, nonatomic, readwrite, retain) A *nonnullABoth;
+@end
+
+#import 
+
+extern int *global_int_ptr;
+
+int *global_int_fun(int *ptr, int *ptr2);
+
+#define SOMEKIT_DOUBLE double
+
+__attribute__((objc_root_class))
+@interface OverriddenTypes
+-(int *)methodToMangle:(int *)ptr1 second:(int *)ptr2;
+@property int *intPropertyToMangle;
+@end
+
+@interface A(ImplicitGetterSetters)
+@property (nonatomic, readonly, retain) A *implicitGetOnlyInstance;
+@property (class, nonatomic, readonly, retain) A *implicitGetOnlyClass;
+
+@property (nonatomic, readwrite, retain) A *implicitGetSetInstance;
+@property (class, nonatomic, readwrite, retain) A *implicitGetSetClass;
+@end
+
+#endif
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
@@ -0,0 +1,98 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector:"transform:"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "anything but this"
+  - Selector: "transform:integer:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+  - Selector: "implicitGetOnlyInstance"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetOnlyClass"
+MethodKind:  Class
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetSetInstance"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetSetClass"
+MethodKind:  Class
+Availability:none
+AvailabilityMsg: "getter gone"

[PATCH] D88446: docs: add documentation describing API Notes

2020-10-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith accepted this revision.
rsmith added a comment.

Thanks, looks good to me.




Comment at: clang/docs/APINotes.rst:250-252
+  Note that the type is *not* parsed in the context where it will be used,
+  which means that macros are not available and nullability must be applied
+  explicitly (even in an ``NS_ASSUME_NONNULL_BEGIN`` section).

Please update this text similarly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D88446: docs: add documentation describing API Notes

2020-10-02 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd updated this revision to Diff 295924.
compnerd marked an inline comment as done.
compnerd added a comment.

Address feedback from @rsmith


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

Files:
  clang/docs/APINotes.rst
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
  clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes

Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
@@ -0,0 +1,15 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector: "privateTransform:input:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+Properties:
+  - Name: internalProperty
+Nullability: N
+Protocols:
+  - Name: InternalProtocol
+Availability: none
+AvailabilityMsg: "not for you"
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
@@ -0,0 +1,4 @@
+@interface A(ExplicitNullabilityProperties)
+@property (nonatomic, readwrite, retain, nonnull) A *explicitNonnullInstance;
+@property (nonatomic, readwrite, retain, nullable) A *explicitNullableInstance;
+@end
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
@@ -0,0 +1,60 @@
+#ifndef SOMEKIT_H
+#define SOMEKIT_H
+
+__attribute__((objc_root_class))
+@interface A
+-(A*)transform:(A*)input;
+-(A*)transform:(A*)input integer:(int)integer;
+
+@property (nonatomic, readonly, retain) A* someA;
+@property (nonatomic, retain) A* someOtherA;
+
+@property (nonatomic) int intValue;
+@end
+
+@interface B : A
+@end
+
+@interface C : A
+- (instancetype)init;
+- (instancetype)initWithA:(A*)a;
+@end
+
+@interface ProcessInfo : A
++(instancetype)processInfo;
+@end
+
+@interface A(NonNullProperties)
+@property (nonatomic, readwrite, retain) A *nonnullAInstance;
+@property (class, nonatomic, readwrite, retain) A *nonnullAInstance;
+
+@property (nonatomic, readwrite, retain) A *nonnullAClass;
+@property (class, nonatomic, readwrite, retain) A *nonnullAClass;
+
+@property (nonatomic, readwrite, retain) A *nonnullABoth;
+@property (class, nonatomic, readwrite, retain) A *nonnullABoth;
+@end
+
+#import 
+
+extern int *global_int_ptr;
+
+int *global_int_fun(int *ptr, int *ptr2);
+
+#define SOMEKIT_DOUBLE double
+
+__attribute__((objc_root_class))
+@interface OverriddenTypes
+-(int *)methodToMangle:(int *)ptr1 second:(int *)ptr2;
+@property int *intPropertyToMangle;
+@end
+
+@interface A(ImplicitGetterSetters)
+@property (nonatomic, readonly, retain) A *implicitGetOnlyInstance;
+@property (class, nonatomic, readonly, retain) A *implicitGetOnlyClass;
+
+@property (nonatomic, readwrite, retain) A *implicitGetSetInstance;
+@property (class, nonatomic, readwrite, retain) A *implicitGetSetClass;
+@end
+
+#endif
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
@@ -0,0 +1,98 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector:"transform:"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "anything but this"
+  - Selector: "transform:integer:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+  - Selector: "implicitGetOnlyInstance"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetOnlyClass"
+MethodKind:  Class
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetSetInstance"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetSetClass"
+MethodKind:  Class
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "setImplicitGetSetInstance:"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "setter gone"
+  - 

[PATCH] D88446: docs: add documentation describing API Notes

2020-10-02 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd marked 2 inline comments as done.
compnerd added inline comments.



Comment at: clang/docs/APINotes.rst:233-235
+  Note that the type is *not* parsed in the context where it will be used,
+  which means that macros are not available and nullability must be applied
+  explicitly (even in an ``NS_ASSUME_NONNULL_BEGIN`` section).

rsmith wrote:
> compnerd wrote:
> > rsmith wrote:
> > > So what context is it parsed in? Below you have an `NSArray *` example; 
> > > how do we do the lookup for `NSArray`?
> > A separate buffer is constructed where the annotations are processed.  
> > During semantic analysis, the requested APINotes are processed and the 
> > attributes specified are applied to the declaration.
> I've not been able to work out what the rule is, based on this documentation. 
> The name `NSArray` isn't predeclared, so how is it found? How can a user 
> ensure their types are visible to this parsing process? Are these things 
> parsed as if they appear at the global scope in a translation unit in which 
> the corresponding framework has been imported? (And if so, why wouldn't 
> macros be available there too?) Whatever the rule is, I'd like to see it 
> documented.
Discussed offline, this has been updated.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D88446: docs: add documentation describing API Notes

2020-10-02 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added inline comments.



Comment at: clang/docs/APINotes.rst:233-235
+  Note that the type is *not* parsed in the context where it will be used,
+  which means that macros are not available and nullability must be applied
+  explicitly (even in an ``NS_ASSUME_NONNULL_BEGIN`` section).

compnerd wrote:
> rsmith wrote:
> > So what context is it parsed in? Below you have an `NSArray *` example; how 
> > do we do the lookup for `NSArray`?
> A separate buffer is constructed where the annotations are processed.  During 
> semantic analysis, the requested APINotes are processed and the attributes 
> specified are applied to the declaration.
I've not been able to work out what the rule is, based on this documentation. 
The name `NSArray` isn't predeclared, so how is it found? How can a user ensure 
their types are visible to this parsing process? Are these things parsed as if 
they appear at the global scope in a translation unit in which the 
corresponding framework has been imported? (And if so, why wouldn't macros be 
available there too?) Whatever the rule is, I'd like to see it documented.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D88446: docs: add documentation describing API Notes

2020-10-02 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd updated this revision to Diff 295835.
compnerd added a comment.

Update text based on feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

Files:
  clang/docs/APINotes.rst
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
  clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes

Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
@@ -0,0 +1,15 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector: "privateTransform:input:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+Properties:
+  - Name: internalProperty
+Nullability: N
+Protocols:
+  - Name: InternalProtocol
+Availability: none
+AvailabilityMsg: "not for you"
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
@@ -0,0 +1,4 @@
+@interface A(ExplicitNullabilityProperties)
+@property (nonatomic, readwrite, retain, nonnull) A *explicitNonnullInstance;
+@property (nonatomic, readwrite, retain, nullable) A *explicitNullableInstance;
+@end
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
@@ -0,0 +1,60 @@
+#ifndef SOMEKIT_H
+#define SOMEKIT_H
+
+__attribute__((objc_root_class))
+@interface A
+-(A*)transform:(A*)input;
+-(A*)transform:(A*)input integer:(int)integer;
+
+@property (nonatomic, readonly, retain) A* someA;
+@property (nonatomic, retain) A* someOtherA;
+
+@property (nonatomic) int intValue;
+@end
+
+@interface B : A
+@end
+
+@interface C : A
+- (instancetype)init;
+- (instancetype)initWithA:(A*)a;
+@end
+
+@interface ProcessInfo : A
++(instancetype)processInfo;
+@end
+
+@interface A(NonNullProperties)
+@property (nonatomic, readwrite, retain) A *nonnullAInstance;
+@property (class, nonatomic, readwrite, retain) A *nonnullAInstance;
+
+@property (nonatomic, readwrite, retain) A *nonnullAClass;
+@property (class, nonatomic, readwrite, retain) A *nonnullAClass;
+
+@property (nonatomic, readwrite, retain) A *nonnullABoth;
+@property (class, nonatomic, readwrite, retain) A *nonnullABoth;
+@end
+
+#import 
+
+extern int *global_int_ptr;
+
+int *global_int_fun(int *ptr, int *ptr2);
+
+#define SOMEKIT_DOUBLE double
+
+__attribute__((objc_root_class))
+@interface OverriddenTypes
+-(int *)methodToMangle:(int *)ptr1 second:(int *)ptr2;
+@property int *intPropertyToMangle;
+@end
+
+@interface A(ImplicitGetterSetters)
+@property (nonatomic, readonly, retain) A *implicitGetOnlyInstance;
+@property (class, nonatomic, readonly, retain) A *implicitGetOnlyClass;
+
+@property (nonatomic, readwrite, retain) A *implicitGetSetInstance;
+@property (class, nonatomic, readwrite, retain) A *implicitGetSetClass;
+@end
+
+#endif
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
@@ -0,0 +1,98 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector:"transform:"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "anything but this"
+  - Selector: "transform:integer:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+  - Selector: "implicitGetOnlyInstance"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetOnlyClass"
+MethodKind:  Class
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetSetInstance"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetSetClass"
+MethodKind:  Class
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "setImplicitGetSetInstance:"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "setter gone"
+  - Selector: "setImplicitGetSetClass:"
+

[PATCH] D88446: docs: add documentation describing API Notes

2020-10-02 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd marked an inline comment as done.
compnerd added inline comments.



Comment at: clang/docs/APINotes.rst:216
+Due to a compiler bug, 'NullabilityOfRet' may change nullability of the
+parameters as well (rdar://30544062). Avoid using it and instead use
+'ResultType' and specify the return type along with a nullability

hlopko wrote:
> Could we get more context from the Apple bug tracker? Depending on the 
> details it might be reasonable to drop NullabilityOfRet attribute completely, 
> or more precisely document when it should be used.
The bug is that Nullability and Type on Parameters entries don’t really 
compose. Consider the following note:

```
  NullabilityOfRet: N
  Parameters:
  - Position: 1
Nullability: O
Type: 'NSDictionary *'
```

on a method: this parameter won’t be treated as optional. Effectively, you need 
to write the _Nullable on the type because the Nullability entry is ignored.

I think that fixing this later is preferable rather than earlier to ensure that 
we at least have a unified starting point where clang and Swift can co-evolve.  
If we try to do that while upstreaming, it will make everything far more 
difficult because you have to coordinate over multiple (3+) repositories.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D88446: docs: add documentation describing API Notes

2020-10-02 Thread Marcel Hlopko via Phabricator via cfe-commits
hlopko accepted this revision.
hlopko added a comment.
This revision is now accepted and ready to land.

Thanks for pushing this forward!

Current proposal seems to be only dealing with annotating ObjC for Swift, but 
API notes seem like a feature that would be useful for other languages as well. 
I briefly chatted with gribozavr and we concluded that if the need arises, the 
current design should be extensible to e.g. C++ (one of the bigger problems 
being how to name C++ function/overload sets).




Comment at: clang/docs/APINotes.rst:187-190
+  - "N"onnull (``_Nonnull``)
+  - "O"ptional (``_Nullable``)
+  - "U"nspecified (``_Null_unspecified``)
+  - "S"calar (deprecated)

compnerd wrote:
> rsmith wrote:
> > Is it important that these are single letters? Spelling out the name in 
> > full (as you do for other enumerated values like `MethodKind` and 
> > `PropertyKind`) would seem a little more readable. (We could accept the 
> > single-letter forms as aliases.)
> I don't think that they be single letters is important.  As long as we have 
> the compatibility aliases, I think it should be fine to support the fully 
> spelt out versions, as the compatibility is needed for existing APINotes.
+1, this is the only attribute that is abbreviated.



Comment at: clang/docs/APINotes.rst:216
+Due to a compiler bug, 'NullabilityOfRet' may change nullability of the
+parameters as well (rdar://30544062). Avoid using it and instead use
+'ResultType' and specify the return type along with a nullability

Could we get more context from the Apple bug tracker? Depending on the details 
it might be reasonable to drop NullabilityOfRet attribute completely, or more 
precisely document when it should be used.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D88446: docs: add documentation describing API Notes

2020-10-01 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd marked 3 inline comments as done.
compnerd added inline comments.



Comment at: clang/docs/APINotes.rst:45
+Clang will search for API notes files next to module maps only when passed the
+``-fapinotes-modules`` option.
+

rsmith wrote:
> Can we add a hyphen between `api` and `notes` here? `-fapinotes` is a little 
> hard to read.
Sure; we can always add a `-fapinotes-modules` as a silent alias for backwards 
compatibility.  I assume you would like the same for the rest of the options, 
and have made that change through out.



Comment at: clang/docs/APINotes.rst:187-190
+  - "N"onnull (``_Nonnull``)
+  - "O"ptional (``_Nullable``)
+  - "U"nspecified (``_Null_unspecified``)
+  - "S"calar (deprecated)

rsmith wrote:
> Is it important that these are single letters? Spelling out the name in full 
> (as you do for other enumerated values like `MethodKind` and `PropertyKind`) 
> would seem a little more readable. (We could accept the single-letter forms 
> as aliases.)
I don't think that they be single letters is important.  As long as we have the 
compatibility aliases, I think it should be fine to support the fully spelt out 
versions, as the compatibility is needed for existing APINotes.



Comment at: clang/docs/APINotes.rst:233-235
+  Note that the type is *not* parsed in the context where it will be used,
+  which means that macros are not available and nullability must be applied
+  explicitly (even in an ``NS_ASSUME_NONNULL_BEGIN`` section).

rsmith wrote:
> So what context is it parsed in? Below you have an `NSArray *` example; how 
> do we do the lookup for `NSArray`?
A separate buffer is constructed where the annotations are processed.  During 
semantic analysis, the requested APINotes are processed and the attributes 
specified are applied to the declaration.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D88446: docs: add documentation describing API Notes

2020-09-29 Thread Richard Smith - zygoloid via Phabricator via cfe-commits
rsmith added a comment.

Broadly, it seems reasonable to me for Clang to support this. I have no major 
concerns with the overall approach here, and it seems like you already have 
sufficient implementation experience with this approach to know that it's going 
to work out well in practice. I'm happy that all of the points in 
http://clang.llvm.org/get_involved.html for Clang extensions are covered -- or 
will be covered by the work to implement this.




Comment at: clang/docs/APINotes.rst:5-12
+**The Problem:** You have headers you want to use, but you also want to add
+extra information to some of the APIs. You don't want to put that information
+in the headers themselves --- perhaps because you want to keep them clean for
+other clients, or perhaps because they're from some open source project and you
+don't want to modify them at all.
+
+**Incomplete solution:** Redeclare all the interesting APIs in your own header

"API" should generally be used only as an uncountable noun. If you mean 
something more concrete here, such as "a function, method, type, ...", then 
saying that would be better, since "API" can be interpreted as meaning an 
individual such entity or a collection of such entities, depending on the 
reader.



Comment at: clang/docs/APINotes.rst:45
+Clang will search for API notes files next to module maps only when passed the
+``-fapinotes-modules`` option.
+

Can we add a hyphen between `api` and `notes` here? `-fapinotes` is a little 
hard to read.



Comment at: clang/docs/APINotes.rst:187-190
+  - "N"onnull (``_Nonnull``)
+  - "O"ptional (``_Nullable``)
+  - "U"nspecified (``_Null_unspecified``)
+  - "S"calar (deprecated)

Is it important that these are single letters? Spelling out the name in full 
(as you do for other enumerated values like `MethodKind` and `PropertyKind`) 
would seem a little more readable. (We could accept the single-letter forms as 
aliases.)



Comment at: clang/docs/APINotes.rst:233-235
+  Note that the type is *not* parsed in the context where it will be used,
+  which means that macros are not available and nullability must be applied
+  explicitly (even in an ``NS_ASSUME_NONNULL_BEGIN`` section).

So what context is it parsed in? Below you have an `NSArray *` example; how do 
we do the lookup for `NSArray`?



Comment at: clang/docs/APINotes.rst:273
+  Used for ``NSError`` code enums. The value is the name of the associated 
domain
+  ``NSString`` constant; an empty string ("") means the enum is a normal enum
+  rather than an error code.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88446

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D88446: docs: add documentation describing API Notes

2020-09-28 Thread Saleem Abdulrasool via Phabricator via cfe-commits
compnerd created this revision.
compnerd added reviewers: MForster, gribozavr2, rsmith.
Herald added a subscriber: jfb.
Herald added a project: clang.
compnerd requested review of this revision.

API Notes are a feature which allows annotation of headers by an
auxiliary file that contains metadata for declarations pertaining to the
associated module.  This enables adding attributes to declarations
without requiring modification of the headers, enabling finer grained
control for library headers for consumers without having to modify
external headers.

Based on the work from 
https://github.com/llvm/llvm-project-staging/commit/d238492fd3a82160955d0642b85d8ce620fa3258


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D88446

Files:
  clang/docs/APINotes.rst
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
  clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
  
clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes

Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit_private.apinotes
@@ -0,0 +1,15 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector: "privateTransform:input:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+Properties:
+  - Name: internalProperty
+Nullability: N
+Protocols:
+  - Name: InternalProtocol
+Availability: none
+AvailabilityMsg: "not for you"
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKitExplicitNullability.h
@@ -0,0 +1,4 @@
+@interface A(ExplicitNullabilityProperties)
+@property (nonatomic, readwrite, retain, nonnull) A *explicitNonnullInstance;
+@property (nonatomic, readwrite, retain, nullable) A *explicitNullableInstance;
+@end
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.h
@@ -0,0 +1,60 @@
+#ifndef SOMEKIT_H
+#define SOMEKIT_H
+
+__attribute__((objc_root_class))
+@interface A
+-(A*)transform:(A*)input;
+-(A*)transform:(A*)input integer:(int)integer;
+
+@property (nonatomic, readonly, retain) A* someA;
+@property (nonatomic, retain) A* someOtherA;
+
+@property (nonatomic) int intValue;
+@end
+
+@interface B : A
+@end
+
+@interface C : A
+- (instancetype)init;
+- (instancetype)initWithA:(A*)a;
+@end
+
+@interface ProcessInfo : A
++(instancetype)processInfo;
+@end
+
+@interface A(NonNullProperties)
+@property (nonatomic, readwrite, retain) A *nonnullAInstance;
+@property (class, nonatomic, readwrite, retain) A *nonnullAInstance;
+
+@property (nonatomic, readwrite, retain) A *nonnullAClass;
+@property (class, nonatomic, readwrite, retain) A *nonnullAClass;
+
+@property (nonatomic, readwrite, retain) A *nonnullABoth;
+@property (class, nonatomic, readwrite, retain) A *nonnullABoth;
+@end
+
+#import 
+
+extern int *global_int_ptr;
+
+int *global_int_fun(int *ptr, int *ptr2);
+
+#define SOMEKIT_DOUBLE double
+
+__attribute__((objc_root_class))
+@interface OverriddenTypes
+-(int *)methodToMangle:(int *)ptr1 second:(int *)ptr2;
+@property int *intPropertyToMangle;
+@end
+
+@interface A(ImplicitGetterSetters)
+@property (nonatomic, readonly, retain) A *implicitGetOnlyInstance;
+@property (class, nonatomic, readonly, retain) A *implicitGetOnlyClass;
+
+@property (nonatomic, readwrite, retain) A *implicitGetSetInstance;
+@property (class, nonatomic, readwrite, retain) A *implicitGetSetClass;
+@end
+
+#endif
Index: clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
===
--- /dev/null
+++ clang/test/APINotes/Inputs/Frameworks/SomeKit.framework/Headers/SomeKit.apinotes
@@ -0,0 +1,98 @@
+Name: SomeKit
+Classes:
+  - Name: A
+Methods:
+  - Selector:"transform:"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "anything but this"
+  - Selector: "transform:integer:"
+MethodKind:  Instance
+NullabilityOfRet: N
+Nullability:  [ N, S ]
+  - Selector: "implicitGetOnlyInstance"
+MethodKind:  Instance
+Availability:none
+AvailabilityMsg: "getter gone"
+  - Selector: "implicitGetOnlyClass"
+MethodKind:  Class
+Availability:none
+AvailabilityMsg: "getter gone"