[cfe-dev] cppcoreguidelines-pro-type-member-init

[Felix Berger via cfe-dev]

Is there an outstanding review I can take a look at?

On Mon, Mar 28, 2016 at 4:47 PM, Michael Miller <michael_miller at motu.com>
wrote:

> Any thoughts on this before I submit what I've got for review? I've now
> run it against our entire codebase and it seems to be behaving as expected.
>
> Thanks!
> Michael Miller
>
> On Mon, Mar 14, 2016 at 9:04 AM, Michael Miller <michael_miller at motu.com>
> wrote:
>
>> Thanks, Felix and Alex! I'm basically done updating the check and
>> expanding the tests.
>>
>> One issue I've run into is that if you have multiple constructors on a
>> class, it's very possible to get warnings about initializing the same
>> member in each. In that case, we end up adding multiple sets of
>> initialization braces for the in-class initializer. I'm not sure the best
>> way to deduplicate those. clang-tidy will handle it if the warnings are
>> identical but that's not really possible here while also pointing out the
>> missing members in individual constructors.
>>
>> The other issue/decision has more to do with the specific wording of the
>> C++ Core Guideline, Type.6, that I alluded to in the first email. Types
>> with non-trivial default constructors present a slight conundrum, e.g.:
>>
>> struct Foo {
>>   int Bar;
>>   std::string Baz;
>> };
>>
>> I see three choices that will defend against uninitialized values.
>> 1. Add a default constructor that initializes Bar.
>> 2. Initialize Bar in-class (C++11 only).
>> 3. Initialize Foo wherever it's used.
>>
>> 1 and 2 seem to more closely match the wording of Type.6, unless we
>> interpret "trivially constructible type" to mean any type without a
>> user-provided default constructor.
>>
>> Should I submit what I've got and we can discuss there or is it worth
>> addressing these questions before I do that?
>>
>> Thanks!
>> Michael Miller
>>
>> On Wed, Mar 2, 2016 at 8:57 AM, Felix Berger <flx at google.com> wrote:
>>
>>> Hi Michael,
>>>
>>> thanks for looking into this! Feel free to improve the existing check.
>>> While I would like to work on it more I'll be busy the next couple of weeks
>>> and the existing check already addresses the main issue I've seen in my
>>> code base.
>>>
>>> Cheers,
>>> Felix
>>>
>>> On Wed, Mar 2, 2016 at 10:54 AM, Alexander Kornienko <alexfh at google.com>
>>> wrote:
>>>
>>>> The author of the check (cc-ed) had plans to work on it further, IIUC.
>>>>
>>>> -- Alex
>>>>
>>>> On Fri, Feb 26, 2016 at 8:49 PM, Michael Miller via cfe-dev <
>>>> cfe-dev at lists.llvm.org> wrote:
>>>>
>>>>> Hi all!
>>>>>
>>>>> I just finished writing a clang-tidy check to flag uninitialized
>>>>> members variables. I was thus very happy to see the
>>>>> cppcoreguidelines-pro-type-member-init check land in clang-tidy mainline
>>>>> recently! The documentation for this clang-tidy check implies it's
>>>>> targeting "Type.6: Always initialize a member variable." It seems like some
>>>>> aspects might be missing, though, and I was wondering if there's anyone
>>>>> working on adding them or if the plan is to keep the current check as is.
>>>>>
>>>>> Type.6's enforcement section reads:
>>>>> • Issue a diagnostic for any constructor of a
>>>>> non-trivially-constructible type that does not initialize all member
>>>>> variables. To fix: Write a data member initializer, or mention it in the
>>>>> member initializer list.
>>>>> • Issue a diagnostic when constructing an object of a trivially
>>>>> constructible type without () or {} to initialize its members. To fix: Add
>>>>> () or {}.
>>>>>
>>>>> Right now, the clang-tidy check only seems to address the first bullet
>>>>> point yet only partially. It flags member variables that are built-ins or
>>>>> pointers. Based on my reading, it needs to flag any trivially constructible
>>>>> typed members, not just built-ins and pointers, excepting fully in-class
>>>>> initialized record types. If the second bullet point were addressed, either
>>>>> in this check or a separate one, the two together should cover the full
>>>>> text.
>>>>>
>>>>> The other thing I wonder about is whether trivially constructible is
>>>>> too narrow. There are types that are not trivially constructible, at least
>>>>> in the definition that I know, that still should be flagged:
>>>>>
>>>>> struct Foo {
>>>>>   int Bar;
>>>>>   std::string Baz;
>>>>> };
>>>>>
>>>>> Foo is not trivially constructible because Baz has a user defined
>>>>> constructor, yet a default initialized Foo will leave Bar in an
>>>>> indeterminate state. Therefore, it seems like we need to initialize any
>>>>> member that is one of the following:
>>>>> A. Built-in or pointer type
>>>>> B. Record type without a user-provided default constructor with at
>>>>> least one member that needs to be initialized as per these criteria
>>>>> C. Array of any type matching these criteria*
>>>>>
>>>>> Is anyone actively working on these aspects of Type.6 or is that
>>>>> something that I should look into?
>>>>>
>>>>> * One might prefer arrays to be default initialized for performance
>>>>> reasons in some cases. Perhaps this should be an option.
>>>>>
>>>>> Thanks!
>>>>> Michael Miller
>>>>>
>>>>> _______________________________________________
>>>>> cfe-dev mailing list
>>>>> cfe-dev at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>>>>>
>>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160328/4bf1f4c3/attachment.html>