Tune inspections to a specific C++ standard

[Quuxplusone]

Bugzilla Link	PR45045
Status	NEW
Importance	P enhancement
Reported by	Zvi Tarem (zvi.tarem@gmail.com)
Reported on	2020-02-27 07:41:08 -0800
Last modified on	2020-03-03 09:34:49 -0800
Version	unspecified
Hardware	PC All
CC	alexfh@google.com, david.bolvansky@gmail.com, djasper@google.com, klimek@google.com, N.James93@hotmail.co.uk
Fixed by commit(s)
Attachments
Blocks
Blocked by
See also

Please allow specifying the C++ standard for which to inspect.

It is not helpful, and is very distracting, being advised to use 'nullptr', 'auto', 'explicit', 'override', 'move', etc. if the C++ standard for your project is C98!

[Quuxplusone]

Real example?

[Quuxplusone]

I am developing a C++ project targeting gcc 4.3 on SLES 11, which supports -
std=c++03 at best. I use CLion and get clang-tidy inspection assistance.

In the following example I get some useless advice:

#include <string>

class Foo
{
public:
  Foo(std::string name) : m_name(name) {}
  virtual ~Foo() {}

protected:
  std::string m_name;
};

1. Use 'explicit' for the constructor.
2. use '= default' for the destructor.
3. use std::move for initializing the m_name member.
4. If I later derive from this class, I am advised to use 'override' on the
destructor.

The CLion folks tell me that clang-tidy has no way to tailor advice based on
the C++ standard of the project, which they do parse out of the CMakeLists.txt
file.

[Quuxplusone]

Each check has its own logic re: supported language standard. If you see results from checks that are irrelevant for the language version your project uses, it's best to turn these checks off.

We can add safeguards for each check (e.g. specifically for google-explicit-constructor, one would need to change CPlusPlus to CPlusPlus11 here: https://github.com/llvm/llvm-project/blob/master/clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp#L24). However, it just doesn't make sense to turn the check on for your project.

BTW, how do you choose the set of checks to run?

[Quuxplusone]

Wait, the explicit keyword for constructors is a C++98 feature. You probably mean suggestions to add explicit to conversion operators (this is a C++11 feature).

Apart from that most checks should have checks for the appropriate standard version. If not, please file separate bugs with an isolated test case (ideally, without any #includes) and a full clang-tidy command line with all compiler options (e.g. clang-tidy file.cc -checks=-*,google-explicit-constructor -- -std=c++98). Each case should be handled separately.

[Quuxplusone]

And make sure your compilations database contains correct -std=c++XX options.

[Quuxplusone]

File bugs for each check that is running in an incompatible version. Checks can be tailored against the version of c++(or other languages) and should disable themselves if running in an incompatible language version.

[Quuxplusone]

The only conclusion I get from this discussion is that there is a break in communication between the JetBrains team developing CLion, and the team developing clang-tidy. As a user of CLion I do not run the clang-tidy command myself, and I have no idea what the settings or flags are, nor how to change them. My cmake projects explicitly set the C++ standard to use, and everything else Should Just Work.

The report I filed with CLion was closed as a 3-rd party problem. You claim that your tool does have the facility to control this issue. I will get back to the CLion team and tell them that.

It seems to me that resolving this issue can be beneficial to both parties, improving overall user experience. This should encourage improving communication between the development teams.

[Quuxplusone]

(In reply to Zvi Tarem from comment #7)
> The report I filed with CLion was closed as a 3-rd party problem. You claim
> that your tool does have the facility to control this issue. I will get back
> to the CLion team and tell them that.

That is not the consensus. CLion should make sure its passing the correct
language version to clang-tidy, likewise some clang-tidy checks should be
modified to ensure they only run on a supported language version.

I have put up a few reviews https://reviews.llvm.org/D75289 and
(child)https://reviews.llvm.org/D75340 to help with the clang-tidy side of
things, but any bug reports of checks ran on incompatible language versions
would be greatly appreciated.

[Quuxplusone]

(In reply to Zvi Tarem from comment #7)
> The only conclusion I get from this discussion is that there is a break in
> communication between the JetBrains team developing CLion, and the team
> developing clang-tidy.  As a user of CLion I do not run the clang-tidy
> command myself, and I have no idea what the settings or flags are, nor how
> to change them. My cmake projects explicitly set the C++ standard to use,
> and everything else Should Just Work.

It's worth understanding the (huge) difference between the CLion team and the
community behind the LLVM project (which clang-tidy is a part of). While the
former is a commercial product with certain support policies, the latter is an
open-source project with a number of contributors interested in particular
features and use cases.

If CLion exposes clang-tidy diagnostics in a certain way and treats it as a 3rd
party tool (i.e. their support doesn't take the burden of investigating the
issues and having them reported and/or fixed), they should educate the users of
the ways to file helpful feedback for clang-tidy and give them tools to gather
relevant diagnostic information in a form convenient for clang-tidy
contributors to reproduce and fix issues. In particular, it should be easy to
get the clang-tidy command line out of the IDE. Users should also be prepared
to create isolated test cases from their code.

> It seems to me that resolving this issue can be beneficial to both parties,
> improving overall user experience.

Users should understand the nature of the project. Individuals and companies,
small and large, are working on the LLVM project. However, they have their own
needs and priorities and are not able to provide reliable and consistent
support for all use cases. While many, if not all, of us would be glad, if
clang-tidy was a pure joy to use, we're limited in the time and effort we can
put into resolving user-reported issues. Thus, filing feedback in a helpful
form is the best way to get your issue resolved. As with almost any software
product, to help you we need a precise description of the issue and an easy way
to reliably reproduce it. Specifically, a self-contained source file, a command
line (independent of your machine's specifics, e.g. local paths) to run clang-
tidy, clang-tidy output and your expectations (and what exactly is the
difference between your expectations and reality). And even with ideal feedback
there's no guarantee that someone will have find time to fix the problem
(though we'll try).

> This should encourage improving communication between the development teams.

If CLion team has enough bandwidth to translate user-reported issues to us, we
would definitely welcome this. Otherwise, they should set proper expectations
and educate their users to work with the LLVM community to get their issues
resolved. I don't think it's reasonable to expect the reverse (i.e. clang-tidy
contributors proactively communicating with CLion developers to resolve issues
CLion users run into).

BTW, contributions are also welcome ;)

[Quuxplusone]

This should go some way to solving your issue https://reviews.llvm.org/D75538