[cfe-dev] clang-tidy attribute enforcement

[Aaron Ballman via cfe-dev]

On Thu, Sep 12, 2019 at 4:16 PM Steve Sudit <steve at beeswax.com> wrote:
>
> You're right: my example just doesn't work because standard attributes (as opposed to compiler-specific ones) are not allowed on the call site. Thanks for pointing that out.
>
> An alternative would be to replace [[out]] with OUT, a precompiler macro which is defined but empty. Given that this is visible to clang-tidy, the semantically-neutral token could perhaps be used as an indicator. (This is similar to what was done with MIDL headers, fwiw.)
>
> Does this seem more plausible? I wonder if clang-tidy is even the right place or if the compiler would have to be involved because it's in a position to match the call to the definition, which clang-tidy might not be.
>
> (Ironically, clang already supports compiler-specific per-parameter attributes whose names are similar to what I'd want: read_only, write_only, read_write.)

You can register that your clang-tidy check would also like to be
notified of macro definitions and expansions by overriding the
registerPPCallbacks function from your check, so that may be more
plausible.  See MoveConstructorInitCheck in clang-tidy for an example
of how to do this.

~Aaron

>
> On Thu, Sep 12, 2019 at 3:26 PM Aaron Ballman <aaron at aaronballman.com> wrote:
>>
>> On Thu, Sep 12, 2019 at 12:33 PM Steve Sudit via cfe-dev
>> <cfe-dev at lists.llvm.org> wrote:
>> >
>> > I have a question about a clang-tidy feature that would be useful to me.
>> >
>> > The feature would be to enforce the presence of an attribute on arguments when it is present in the parameter.
>> >
>> > So, for example, if a function is defined like:
>> >   bool tryGet(const std::string& key, [[out]] std::string& value);
>> >
>> > Then calls to it would need to have a matching `[[out]]` attribute, like:
>> >   if (!directory.tryGet(k, [[out]] v)) {
>> >
>> > If the attribute is omitted at the call site, a warning would be generated. Likewise, if the attribute appeared at the call site, but not at the function declaration, that would generate a warning.
>> >
>> > No deeper enforcement is necessary. Presumably, which attributes would get this enforcement would be configurable.
>> >
>> > Is this or anything similar already supported? If not, could someone please point me towards the steps I'd need to take get it added? This could either be as a feature request for someone else to fulfill, or if necessary, my own contribution to the project.
>>
>> Clang does not have a plugin approach for introducing new attributes,
>> so there is currently not a way for a clang-tidy check to introduce a
>> new attribute. Instead, you'd have to modify clang itself to support
>> the attribute. Making a new parameter attribute isn't too difficult,
>> there are examples in Attr.td you could use. However, the attribute
>> you'd like to use at the call site is not possible -- neither C nor
>> C++ allow attributes at the expression level currently so there's no
>> syntax for what you want to do. That said, you may not require the
>> attribute there anyway. For instance, at the call site you can look to
>> the FunctionDecl for the CallExpr (if any) and inspect its parameters
>> to see if any have the [[out]] attribute and use that information to
>> perform your action within the clang tidy check (perhaps).
>>
>> ~Aaron