[clang-tools-extra] [clang-tidy] Adding an initial version of the "Initialized Class Members" checker. (PR #65189)

via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 6 15:01:24 PDT 2023


adriannistor wrote:

@PiotrZSL @carlosgalvezp 

Piotr, thank you for your feedback!

You are right: if this was the entire checker, it would make no sense to add a new checker.

It was my mistake, I should have added more context and explanations to this PR (doing that now!).

This is only the first PR out of many PRs.

The first PR (and probably a few subsequent PRs) will look like they could have been added to `cppcoreguidelines-pro-type-member-init`.

However, the more we add, the more the two checker will diverge.

I actually considered expanding `cppcoreguidelines-pro-type-member-init`, but based on our current plan (more details next) expanding `cppcoreguidelines-pro-type-member-init` will soon become a lot of `if-then-else` blocks.

I updated the documentation (second commit a few minutes ago) in `cpp-init-class-members.rst`, `ReleaseNotes.rst`, and the .H file to reflect the above (i.e., this is the first commit our of many commits and this checker is under active development).

If you would prefer that we write the checker until the end and then upstream, we would be very glad to do that, just let me know!

We felt it is better to incrementally upstream the checker such that the community can give early feedback.  Additionally, if we develop the checker internally and only upstream the checker at the end, there may be internal assumptions that creep in and then it will be difficult to remove for upstream.  Additionally, porting the internal version to the upstream version in one go may require non-trivial work.

However, either way if fine for us, just let me know!

Regarding the checker specifications (especially as they compare the `cppcoreguidelines-pro-type-member-init`):

We have an internal document that sets the direction and high level requirements for the checker.  We are actively updating the requirements based on data, so writing a public version for that document would be premature (and it will require us to invest time and effort into something that will anyway likely change).  Once that document and the checker and the data are in a stable version, we plan to improve the documentation for requirements.

We also have data that is guiding the low level development.  We are actively gathering and analyzing the data, so our low level plans are also under constant change.  However, you can see the low level requirements captured in the unit tests.  As we implement more, the unit tests will capture that.  Once the checker is in a stable version, we plan to improve the low level public requirements.

TLDR:

- This is the first of many PRs.

- We are happy to do what is the best for you (just let us know!), i.e., either: (1) upstream the checker incrementally with PRs or (2) upstream the checker all at once at the end (though there are tradeoffs, discussed above).

- We are building the high level requirements and the low level implementation details as we go, based on data and analysis that we are currently working on.  We are including some documentation (to capture the high level requirements) and unit tests (to capture the low level requirements) in the PRs.  We plan to update the documentation in a more stable version once the checker, our data, our analysis of our data, and the (high level and low level) requirements are in a more stable and mature shape.

Please let me know what you think of the above plan and any feedback you have!  I would be very glad to work based on your feedback!

Thanks!




https://github.com/llvm/llvm-project/pull/65189


More information about the cfe-commits mailing list