[cfe-dev] Adding Support for APINotes

[David Rector via cfe-dev]

> On Oct 6, 2020, at 9:39 AM, James Y Knight via cfe-dev <cfe-dev at lists.llvm.org> wrote:
> 
> Every time this discussion has recurred, people have noted that a feature like this could be valuable for many use-cases beyond swift. And I don't disagree. But, what we actually have here is far from a generic "extra API information" solution. It has a lot of missing functionality, without even a clear path towards adding it. And, many of the features which it does have are extremely swift-specific. What has been built here is clearly purpose-designed just for Swift/ObjC-interop, and it does not really seem realistic to me to expect this to be able to accomplish people's wider dreams of a generally-useful API sidecar mechanism.
> 
> One of the clearest examples of a feature which is strikingly out-of-place as an upstream feature is the "SwiftVersions" stanza, and correspondingly, -fapi-notes-swift-version command-line argument -- but that's hardly the only such issue. I could come up with ideas on how to avoid needing such a weird thing, but I think that would probably not be productive.
> 
> Because, I think the question which is being actually being implicitly asked in this thread is:
>   Is the Clang community OK with adding this (hefty!) amount of code to support a feature which is currently -- and most likely will remain -- only useful for Swift? Furthermore, is it OK to add essentially as-is, with no ability to significantly affect the design? Incompatible changes would then break the deployed Swift usage, and therefore cannot be accepted.
> 
> Quite possibly the answer to both is indeed "yes". Personally, I am vaguely uncomfortable with this, but not entirely opposed to including it on those grounds.
> 
> Yet, what I definitely do not want to see is this getting pushed upstream _without_ it explicitly being stated that the above is what's happening. We need to acknowledge that what is actually being proposed here is "SwiftAPINotes", not "APINotes", and for people to be OK with that.
> 
> (And, as a sidenote, it also seems unclear to me that Swift really needed APINotes to be put into Clang (even Swift's fork of Clang) vs building the similar functionality into the Swift consumer of the Clang AST. Possibly this was an arbitrary choice made early on. Or maybe there's good reasons why it wouldn't work to do that way which aren't obvious at a quick glance. Bu, I suspect there is no appetite for making such a large design change now, even if it could've been better, so that's not really a useful discussion to have.)

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.

> 
> On Mon, Sep 28, 2020 at 5:10 PM Saleem Abdulrasool via cfe-dev <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> Hi!
> 
> I'd like to revive the effort around merging APINotes support into the llvm.org <http://llvm.org/> repository.  This was previously discussed at http://lists.llvm.org/pipermail/cfe-dev/2017-May/053860.html <http://lists.llvm.org/pipermail/cfe-dev/2017-May/053860.html> nearly 3 years ago.  The overall consensus seemed neutral to slightly positive.  Now that the Swift specific attributes have been merged, the APINotes seem like a good next step towards converging the fork.
> 
> I've put up https://reviews.llvm.org/D88446 <https://reviews.llvm.org/D88446> to add initial documentation on the feature before trying to add the actual implementation with the goal of gathering commits on the technical aspects of the feature.
> 
> Thanks.
> 
> -- 
> Saleem Abdulrasool
> compnerd (at) compnerd (dot) org
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev <https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev>
> _______________________________________________
> 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/20201009/92a95b1b/attachment.html>