[cfe-dev] Adding Support for APINotes

[Gábor Márton via cfe-dev]

+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.
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?

Gabor

On Mon, Oct 12, 2020 at 5:51 PM James Y Knight via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

>
>
> On Fri, Oct 9, 2020 at 10:21 AM David Rector <davrecthreads at gmail.com>
> wrote:
>
>> IIUC APINotes wants to add attributes during parsing so that the those
>> attributes are then treated just like real parsed attributes would be by
>> Sema.
>>
>> Ideally this sort of thing should be able to be contained in an
>> ASTConsumer, but it presently cannot: the only real option ASTConsumer
>> provides is to traverse the AST post-parsing in an override of
>> `HandleTranslationUnit()`.  This leaves APINotes no choice but to add its
>> details directly to Sema and Parser, with the result that Swift-specific
>> details would necessarily show up in the guts of mainline clang were this
>> to be upstreamed.
>>
>> If ASTConsumer instead provided a comprehensive virtual interface (but
>> with negligible overhead, somehow) to perform custom consumer actions at
>> strategic points during parsing/Sema (e.g., call consumer virtuals in each
>> Sema::ActOn*), this conflict probably could be, or at least could have
>> been, avoided.
>>
>> Note too that this issue resonates with one a user faced just the other
>> day: he wanted to call `Sema::LookupName` within an ASTMatchers callback
>> (during HandleTranslationUnit, as usual) to determine if a certain node
>> could be rewritten w/o changing its semantics — but of course the necessary
>> Sema/Parser scope info is long gone by that point.  If only he had the
>> option to perform his logic on the node immediately after it was parsed,
>> there would be no issue. Because he cannot, his only option was to
>> laboriously recreate that state data in order to call LookupName.
>>
>> In sum, the dearth of ASTConsumer virtuals hinder encapsulation, causing
>> 1) Sema and Parser to be polluted with what should be a consumer’s details,
>> and 2) consumers to be polluted with what should be Sema & Parser details,
>> and making it difficult to upstream useful tools.
>>
>
> This is an *excellent* point!
>
> It looks like the primary change in Clang which is required for APINotes
> is to place "ProcessAPINotes" calls into each of the Sema::ActOn* functions
> which creates a (subclass of) Decl. This needs to occur after creating the
> Decl object and processing the attributes in the source, but prior to
> actually acting on it (e.g. see apple's SemaDeclObjC.cpp
> <https://github.com/apple/llvm-project/blob/apple/main/clang/lib/Sema/SemaDeclObjC.cpp>).
> This call could instead be a call to a generic hook for modifying the decl,
> rather than specifically for processing API notes. We already even have a
> container for such hooks in Sema, "ExternalSemaSource
> <https://github.com/llvm/llvm-project/blob/master/clang/include/clang/Sema/ExternalSemaSource.h>",
> which seems like a reasonable place to add a virtual method which may
> modify newly-created Decls.
>
> If such a hook mechanism is added, it then seems entirely plausible that
> SemaAPINotes.cpp
> <https://github.com/apple/llvm-project/blob/apple/main/clang/lib/Sema/SemaAPINotes.cpp> and
> lib/APINotes/*
> <https://github.com/apple/llvm-project/tree/apple/main/clang/lib/APINotes> can
> be moved from the clang-fork into swift itself without requiring
> significant modifications of the code, or any change in functionality of
> the feature.
>
> If this works out how it seems like it could, this seems like it may even
> be a better outcome for all parties (both the potential static analysis
> tooling use-cases, and for Swift).
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20201012/39c75c66/attachment.html>