[Clang][Diagnostics] Consider splitting warning on cv-qualified base classes into a separate group

[MagentaTreehouse]

In PR #121419, warnings were added for cv-qualified base classes. I consider it too noisy in some scenarios. Consider this code:

#include <type_traits>

inline constexpr auto op0{[] { /* do work */ }};
struct A: decltype(op0) { /* members */ };
inline constexpr auto op1{[] { /* do work */ }};
struct B: decltype(op1) { /* members */ };
// and on and on and on...
// and on and on and on...

// fix: =[
struct C: std::remove_cv_t<decltype(op1)> { /* members */ };
// and on and on and on...

Warnings:

<source>:4:11: warning: 'const' qualifier on base class type 'decltype(op0)' (aka 'const (lambda at <source>:3:27)') has no effect [-Wignored-qualifiers]
    4 | struct A: decltype(op0) { /* members */ };
      |           ^
<source>:4:11: note: base class 'decltype(op0)' (aka 'const (lambda at <source>:3:27)') specified here
<source>:6:11: warning: 'const' qualifier on base class type 'decltype(op1)' (aka 'const (lambda at <source>:5:27)') has no effect [-Wignored-qualifiers]
    6 | struct B: decltype(op1) { /* members */ };
      |           ^
<source>:6:11: note: base class 'decltype(op1)' (aka 'const (lambda at <source>:5:27)') specified here

and on and on and on...

The const comes from decltyping from constexpr variables. I would argue that the language rule is doing me a favor here, so that I don't have to add unnecessary verbosity by specifying std::remove_cv_t to each one of them.

It would be nice to split this warning into a separate group, so that it is not enabled by -Wextra, or that at least I can turn this specific warning off.

CC @a-tarasyuk @erichkeane

[erichkeane]

I think that putting this diagnostic in its own warning group, still in -Wextra is a perfectly reasonable request.

I also believe there is value to only diagnosing this on EXPLICIT qualifiers (or a separate, additional warning to do so), not ones that get picked up from the template.

I think the two of these could reasonably be done together, and should be a fairly minor amount of effort, so I'll mark this as a good beginner bug.

[llvmbot]

Hi!

This issue may be a good introductory issue for people new to working on LLVM. If you would like to work on this issue, your first steps are:

1. Check that no other contributor has already been assigned to this issue. If you believe that no one is actually working on it despite an assignment, ping the person. After one week without a response, the assignee may be changed.
2. In the comments of this issue, request for it to be assigned to you, or just create a pull request after following the steps below. Mention this issue in the description of the pull request.
3. Fix the issue locally.
4. Run the test suite locally. Remember that the subdirectories under test/ create fine-grained testing targets, so you can e.g. use make check-clang-ast to only run Clang's AST tests.
5. Create a Git commit.
6. Run git clang-format HEAD~1 to format your changes.
7. Open a pull request to the upstream repository on GitHub. Detailed instructions can be found in GitHub's documentation. Mention this issue in the description of the pull request.

If you have any further questions about this issue, don't hesitate to ask via a comment in the thread below.

[llvmbot]

@llvm/issue-subscribers-good-first-issue

Author: None (MagentaTreehouse)

In PR #121419, warnings were added for cv-qualified base classes. I consider it too noisy in some scenarios. Consider this code: ```c++ #include <type_traits>

inline constexpr auto op0{[] { /* do work / }};
struct A: decltype(op0) { / members / };
inline constexpr auto op1{[] { / do work / }};
struct B: decltype(op1) { / members */ };
// and on and on and on...
// and on and on and on...

// fix: =[
struct C: std::remove_cv_t<decltype(op1)> { /* members */ };
// and on and on and on...

Warnings:
```console
<source>:4:11: warning: 'const' qualifier on base class type 'decltype(op0)' (aka 'const (lambda at <source>:3:27)') has no effect [-Wignored-qualifiers]
    4 | struct A: decltype(op0) { /* members */ };
      |           ^
<source>:4:11: note: base class 'decltype(op0)' (aka 'const (lambda at <source>:3:27)') specified here
<source>:6:11: warning: 'const' qualifier on base class type 'decltype(op1)' (aka 'const (lambda at <source>:5:27)') has no effect [-Wignored-qualifiers]
    6 | struct B: decltype(op1) { /* members */ };
      |           ^
<source>:6:11: note: base class 'decltype(op1)' (aka 'const (lambda at <source>:5:27)') specified here

and on and on and on...

The const comes from decltyping from constexpr variables. I would argue that the language rule is doing me a favor here, so that I don't have to add unnecessary verbosity by specifying std::remove_cv_t to each one of them.

It would be nice to split this warning into a separate group, so that it is not enabled by -Wextra, or that at least I can turn this specific warning off.

CC @a-tarasyuk @erichkeane

[lyledean1]

I’d like to work on this issue - could you assign me @MagentaTreehouse

[erichkeane]

I’d like to work on this issue - could you assign me @MagentaTreehouse

We haven't been using the 'assignment' tool in Github lately for these issues, and instead are just counting on comments like that (as otherwise it dissuades anyone from looking into this if you haven't shown progress in a while).

If you find yourself needing advice/etc, please let us know! Also, make sure to add me to the review if you get one together, even if it is incomplete it is a good place to start conversation.

[lyledean1]

Thanks @erichkeane , i've added a draft pr here so can continue the discussion there