[cfe-dev] Adding Support for APINotes

[David Rector via cfe-dev]

> On Oct 13, 2020, at 12:11 PM, Gábor Márton <martongabesz at gmail.com> wrote:
> 
> > Do you think that will actually be useful to the static analyzer use-case? I'd expect that in most use-cases, you'd want to provide the changes via some mechanism other than searching for a file named "APINotes.apinotes" in the same directory as you found the header. Other than for Swift with Apple frameworks, it seems likely to be pretty rare that the producer of a particular library would be the same entity providing a file of extra attributes on the side?
> 
> It might be convenient for a library author to attach a YAML file that describes which functions should be checked e.g. for taint analysis. This file could be placed conveniently right next to the headers. On the other hand, this is not an option for some system libraries (e.g. libc), we definitely don't want libc authors to provide this data, it should be the role of the analyzer developers. Actually we do that today for many standard C/C++ or POSIX functions, but we simply hardcode the information into the source code of the analyzer <https://github.com/llvm/llvm-project/blob/08097fc6a9746370e073e617036847d9ea46c9f4/clang/lib/StaticAnalyzer/Checkers/StdLibraryFunctionsChecker.cpp#L1778> itself.
> 
> > I wouldn't think so, since this mechanism modifies the nodes as part of their initial creation, not as a subsequent mutation?
> 
> My understanding is that we create a Decl and then a bit later we create an Attr and attach that to it, so this seems to be a mutation to me. However, I am not sure, perhaps this should not be interpreted as such.

There seem to be three systematic places a consumer virtual could be called:
Between Parser and Sema, s.t. the user could tweak e.g. the `ParsedAttributes` of a `Declarator D` before `Actions.ActOnDeclarator(D)` is called, or whatever.  (Ugh.)
After Sema is finished acting on a node, but before subsequent parsing.  I.e., on the Decl returned by e.g. `Actions.ActOnDeclarator()`.
That *would* require a call to the ASTMutationListener. (Ugh.)
Immediately after a node is allocated via `*::Create()` in `Sema::ActOn*` etc.  
I.e., these calls would be a bit before the present placement of most `ProcessAPINotes(D)` calls.
#3, James’s solution, seems ideal.  Because Sema cannot possibly have "acted on"/depended on a node which has only just been allocated (right?), this would give the consumer a hook to both a) visit a node *along with* the Parser/Sema state info at the time it was created, and b) tweak how a `FooDecl` is parsed/instantiated before Sema can see it.

All this could be contained in a simple override of a virtual e.g. `VisitCreatedFooDecl(FooDecl *D, bool Parsed1Instantiated0)` exposed in ASTConsumer or ExternalSemaSource.  

APINotes would presumably then be able to move all calls to `ProcessAPINotes(D)` out of Sema and into  `VisitCreatedFunctionDecl(FunctionDecl *D)/VisitCreatedObjCMethodDecl(…)/VisitCreatedParmVarDecl(…)` etc. overrides in its consumer.

If all the premises hold, this would be a broad gain in functionality via a familiar RecursiveASTVisitor-like interface.

> 
> 
> On Mon, Oct 12, 2020 at 9:30 PM James Y Knight <jyknight at google.com <mailto:jyknight at google.com>> wrote:
> On Mon, Oct 12, 2020 at 12:46 PM Gábor Márton <martongabesz at gmail.com <mailto:martongabesz at gmail.com>> wrote:
> +1 for separating the Swift specific Sema logic the way you described.
> However, there are valuable implementations in lib/APINotes/* <https://github.com/apple/llvm-project/tree/apple/main/clang/lib/APINotes> which we could use for a generic APINotes framework in Clang. For instance, APINotesManager::findAPINotes <https://github.com/apple/llvm-project/blob/apple/main/clang/lib/APINotes/APINotesManager.cpp#L325> encapsulates the logic for finding the APINotes files, this seems to be totally independent from Swift.
> 
> Do you think that will actually be useful to the static analyzer use-case? I'd expect that in most use-cases, you'd want to provide the changes via some mechanism other than searching for a file named "APINotes.apinotes" in the same directory as you found the header. Other than for Swift with Apple frameworks, it seems likely to be pretty rare that the producer of a particular library would be the same entity providing a file of extra attributes on the side?
> 
> Also, I believe that particular function is unused in Swift. AFAICT, Swift only uses the modules-based apinotes loading (loadCurrentModuleAPINotes), not the header-path-based loading (that is: it passes "-fapinotes-modules" to its internal Clang invocation, not "-fapinotes").
> 
> A side note: ASTMutationListener <https://clang.llvm.org/doxygen/classclang_1_1ASTMutationListener.html> is the interface which notifies any consumers when an AST node is changed after its creation. I think we should notify all clients once we add a new APINote Attribute (e.g. see AddedAttributeToRecord <https://clang.llvm.org/doxygen/classclang_1_1ASTMutationListener.html#abd04809c814e5ea2af681465004f32fe>), shouldn't we?
> 
> I wouldn't think so, since this mechanism modifies the nodes as part of their initial creation, not as a subsequent mutation?
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20201013/7aad7fb0/attachment.html>