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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to