[cfe-dev] Minor patch controversial

Marcwell Helpdesk via cfe-dev cfe-dev at lists.llvm.org
Fri Mar 3 03:04:10 PST 2017


Hi everyone!

A few weeks back we sent a minor patch to the cfe-commit list that we thought would be a non-issue once it had passed the review phase, but that was not the case, instead we were told that the patch was too controversial so we ask for a general opinion on the matter.

The current implementation of the annotate attribute supports annotation of declarations using the GNU syntax, with its data forwarded into AST and further down to IR. The supported set of declarations includes classes, variables, fields, functions and methods. The patch extends this to annotation of statements and the use of C++11 syntax but with the lack of support in LLVM IR the data is only forwarded onto AST.

Overall, this is a very minor change to what already is present and being used, but the small addition could benefit many plugin and tool developers readily today. The concept of annotations are very generic and makes it possible to insert, for example, control directives or tags directly into the source code that a plugin or tool may read and utilize. Please note that the intention of the patch is to make a small adjustment, not to alter the meaning of annotations into something it is not.

Here is an example usage:

    File: xtool.hpp
    #define XTOOL_A “xtool:directiveA”
    #define XTOOL_B “xtool:directiveB"

    File: sample.cpp
    #include <xlibrary.hpp>

    [[clang::annotate(XTOOL_A]]
    int main(int argc, char* argv[])
    {
        // This is what the patch adds, possibility to annotate a statement
        [[clang::annotate(XTOOL_B)]]
        while( N )
        {  . . .  }
        return 0;
    }

The controversy of the patch appears when comparing annotations with pluggable attributes and suggesting that the two technologies competes for the same goal. No, they do not, for the simple reason that an annotation is an annotation, nothing more, nothing less, and should stay that way. It should not have the same streamlined functionality of PA, such as proper namespacing, argument checking etc. And let us be very clear on one thing; having pluggable attributes would be a great addition to Clang and we’re not trying dissuade anyone from implementing it by promoting annotations instead. The patch does not in any way make annotations move closer to PA than it was before.

One of the complaints in the "annotations vs PA” discussion is how the functionality/information are exposed to both end-users and attribute authors, being error-prone to use. This is a complaint that actually underlines that annotations are not competing with pluggable attributes, it may be fit for some solutions but for others a more strict and controlled environment, like pluggable attributes, may be required. Diversity and different levels of support is what makes Clang superior to the competition. There is no reason to stop using an existing functionality until an alternative is available and to our humble understanding, implementing an architecture that supports PA up front and in both AST and IR probably needs a few iteration to set things straight, pushing the availability date into the distant future.

We ask for this small patch to be committed since it makes a minor enhancement (annotated statements) to an existing functionality that benefits the community of plugin and tool writers today by providing a generic and consistent way of communicating control directives/tags directly from the source code to itself, such as source code transformers, generators, collectors and similar.

Cheers,
Chris




More information about the cfe-dev mailing list