[RFC][clang-tidy] Register warnings as check aliases

Clang Frontend
1

Hi!

I would like to propose that it should be possible to register compiler warnings as clang-tidy check aliases.

As a motivating example, there is a CERT C++ secure coding rule: ERR54-CPP 1

This rule is covered by the clang warning: -Wexceptions

So turning on this check in clang tidy would have two effects: turning on -Wexceptions and display the result of -Wexceptions as ERR54-CPP hits.

In my opinion aliases like this would be a great usability improvement:

  • it would be easier to check the code against some coding guidelines.

  • it would be easier to check what rules are already covered.

  • it would be easier to find uncovered rules to implement.

What do you think? Would you support a feature like that?

Regards,

Gabor

2

Hello!

This sounds useful for those that wants to check CERT compliance.

However if I just run clang-tidy as a complement to get extra checks .. then I don't want that clang compiler warnings are enabled and renamed "behind my back". Do you think we can avoid that?

Many static analyzer checkers has partial coverage of CERT rules also. For instance out-of-bounds, dereferencing null pointers, etc. I assume you would only alias checks that has full coverage?

Imho it would be good to someday have MISRA, JSF, .. checks also. And I assume there is some overlap. so same warning could be aliased by both cert-* and misra-* etc.

Best regards,
Daniel Marjamäki

..................................................................................................................
Daniel Marjamäki Senior Engineer
Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden

Mobile: +46 (0)709 12 42 62
E-mail: Daniel.Marjamaki@evidente.se

www.evidente.se

3

Hi!

I would like to propose that it should be possible to register compiler
warnings as clang-tidy check aliases.

I think this is an excellent idea!

As a motivating example, there is a CERT C++ secure coding rule: ERR54-CPP
[1]

This rule is covered by the clang warning: -Wexceptions

So turning on this check in clang tidy would have two effects: turning on
-Wexceptions and display the result of -Wexceptions as ERR54-CPP hits.

How do you envision the diagnostics being reported? For instance,
would it be [cert-err54-cpp, -Wexceptions], [cert-err54-cpp], or
[-Wexceptions]?

Also, do you envision this overriding a flag if it's disabled? e.g.,
would this diagnose, or silence the diagnostic?

clang-tidy E:\SomeFile.cpp -checks=-*,cert-err54-cpp -- -std=c++14
-Wno-exceptions

In my opinion aliases like this would be a great usability improvement:
- it would be easier to check the code against some coding guidelines.
- it would be easier to check what rules are already covered.
- it would be easier to find uncovered rules to implement.

What do you think? Would you support a feature like that?

I would love to see a feature like this, especially if it's something
users can configure themselves with some sort of file-based
configuration. This degree of flexibility would allow us to more
easily maintain common rulesets like CERT, MISRA, JSF++, C++ Core
Guidelines, etc while still giving users the ability to support custom
rulesets without modifying the Clang source.

~Aaron

4

Hi!

5

> Hi!
>
> I would like to propose that it should be possible to register compiler
> warnings as clang-tidy check aliases.

I think this is an excellent idea!

> As a motivating example, there is a CERT C++ secure coding rule:
ERR54-CPP
> [1]
>
> This rule is covered by the clang warning: -Wexceptions
>
> So turning on this check in clang tidy would have two effects: turning on
> -Wexceptions and display the result of -Wexceptions as ERR54-CPP hits.

How do you envision the diagnostics being reported? For instance,
would it be [cert-err54-cpp, -Wexceptions], [cert-err54-cpp], or
[-Wexceptions]?

I think it should be either [cert-err54-cpp, -Wexceptions] or
[cert-err54-cpp]. In the warning it should be clear that there is a CERT
violation.

Also, do you envision this overriding a flag if it's disabled? e.g.,
would this diagnose, or silence the diagnostic?

clang-tidy E:\SomeFile.cpp -checks=-*,cert-err54-cpp -- -std=c++14
-Wno-exceptions

I would except the tidy flags to be "stronger" and overwrite the
compilation flags. The compilations flags most of the time reflect the
requirements of the builds and not the requirements of the additional
static analysis. What do you think?

> In my opinion aliases like this would be a great usability improvement:
> - it would be easier to check the code against some coding guidelines.
> - it would be easier to check what rules are already covered.
> - it would be easier to find uncovered rules to implement.
>
> What do you think? Would you support a feature like that?

I would love to see a feature like this, especially if it's something
users can configure themselves with some sort of file-based
configuration. This degree of flexibility would allow us to more
easily maintain common rulesets like CERT, MISRA, JSF++, C++ Core
Guidelines, etc while still giving users the ability to support custom
rulesets without modifying the Clang source.

Do you mean registering an alias using a configuration file?

Regards,
Gábor

6

Hello!

However if I just run clang-tidy as a complement to get extra checks .. then I don't want that clang compiler warnings are enabled and renamed "behind my back". Do you think we can avoid that?

What do you mean? In case somebody enables the CERT checks, I think she is likely to be interested in all CERT checks. And right now there, when a check is already covered by a warning it is not added to clang tidy. So in order to get the best coverage on needs to review all of the warnings and enable them explicitly. I think it is much more convenient to just enable the CERT checks and you are good to go. Can you think of a use case where this is unintended behaviour?

The file-based configuration that Aaron suggested will solve my doubts.

Best regards,
Daniel Marjamäki

..................................................................................................................
Daniel Marjamäki Senior Engineer
Evidente ES East AB Warfvinges väg 34 SE-112 51 Stockholm Sweden

Mobile: +46 (0)709 12 42 62
E-mail: Daniel.Marjamaki@evidente.se

www.evidente.se

7

> Hi!
>
> I would like to propose that it should be possible to register compiler
> warnings as clang-tidy check aliases.

I think this is an excellent idea!

> As a motivating example, there is a CERT C++ secure coding rule:
> ERR54-CPP
> [1]
>
> This rule is covered by the clang warning: -Wexceptions
>
> So turning on this check in clang tidy would have two effects: turning
> on
> -Wexceptions and display the result of -Wexceptions as ERR54-CPP hits.

How do you envision the diagnostics being reported? For instance,
would it be [cert-err54-cpp, -Wexceptions], [cert-err54-cpp], or
[-Wexceptions]?

I think it should be either [cert-err54-cpp, -Wexceptions] or
[cert-err54-cpp]. In the warning it should be clear that there is a CERT
violation.

I've been thinking on this, and I think that we should print whatever
identifier is used to enable the diagnostic, because that will help
inform the user how to disable it. So, for instance, if the user runs:

clang-tidy -checks=-*,cert-* SomeFile.cpp -- -Wall

then the diagnostic should show [cert-err54-cpp, -Wexceptions] because
the user would need to disable *both* cert-err54-cpp and -Wexceptions
in order to avoid the diagnostic.

Also, do you envision this overriding a flag if it's disabled? e.g.,
would this diagnose, or silence the diagnostic?

clang-tidy E:\SomeFile.cpp -checks=-*,cert-err54-cpp -- -std=c++14
-Wno-exceptions

I would except the tidy flags to be "stronger" and overwrite the compilation
flags. The compilations flags most of the time reflect the requirements of
the builds and not the requirements of the additional static analysis. What
do you think?

I agree. I think that if the user runs:

clang-tidy -checks=-*,cert-* SomeFile.cpp -- -Wall -Wno-exceptions

then the diagnostic should still be triggered, and show
[cert-err54-cpp, -Wexceptions]. I think the diagnostic should still
list -Wexceptions so that users know all the way the diagnostic can be
enabled, but maybe in an ideal world with color diagnostics, we could
print the -Wexceptions in grey (not even remotely a high priority;
possibly not even a good idea).

If they run:

clang-tidy -checks=-*,cert-dcl* SomeFile.cpp -- -Wall -Wno-exceptions

the diagnostic will be silenced entirely.

> In my opinion aliases like this would be a great usability improvement:
> - it would be easier to check the code against some coding guidelines.
> - it would be easier to check what rules are already covered.
> - it would be easier to find uncovered rules to implement.
>
> What do you think? Would you support a feature like that?

I would love to see a feature like this, especially if it's something
users can configure themselves with some sort of file-based
configuration. This degree of flexibility would allow us to more
easily maintain common rulesets like CERT, MISRA, JSF++, C++ Core
Guidelines, etc while still giving users the ability to support custom
rulesets without modifying the Clang source.

Do you mean registering an alias using a configuration file?

Yes, essentially. It would be nice if I could register a file that
defines all of the mappings for a check category (possibly even a
category that does not currently exist in clang-tidy, such as MISRA).
I'm not quite certain how you would associate this file with
clang-tidy, but I think we should have the ability to have default
mappings that require no user intervention to enable (for instance,
for CERT checks, since we already have a CERT category) that we can
provide, and ad hoc mappings that the user can pass to clang-tidy (for
instance, to enforce some local coding standards on a per-instance
basis).

~Aaron