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

[Michael Miller via cfe-dev]

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/20160314/2b580cc0/attachment.html>