[clang-tidy] `readability-redundant-member-init` vs `-Wmissing-field-initializers`

[dieram3]

I just came across an incompatibility between the new readability-redundant-member-init check and the -Wmissing-field-initializers diagnostic. It is a common pattern to have an aggregate type representing a set of options, some of which are optional, e.g.:

struct options {
    int required_1;
    int required_2;
    std::optional<int> optional{};
};

Now, if I have the following:

void foo(options opts);

void bar() {
    foo({
        .required_1 = 4,
        .required_2 = 2,
        // optional omitted.
    });
}

readability-redundant-member-init will complain the initializer for optional is redundant. However, if I remove it, -Wmissing-field-initializers will complain that the field initializer for optional is missing.

Compiler Explorer repro: https://godbolt.org/z/xfzPv8h47

Is there a way to make these two agree? Or maybe readability-redundant-member-init needs a new option or adjustment?

[chrchr-github]

Not sure how readability-redundant-member-init is related. The initializer is redundant since std::optional has a default constructor.
-Wmissing-field-initializers might need an exception for std::optional members though.

[PiotrZSL]

Ok, I see an issue. {} silents a -Wmissing-designated-field-initializers warning.
This warning were introduced with #81364.

@vvd170501, @AaronBallman
I see that your tests for this warning didn't cover classes.
How this warning should behave in this situation ?

I see that in AST if we do not have {} we end up with:

      `-InitListExpr <col:9, line:16:5> 'options'
        |-IntegerLiteral <line:13:23> 'int' 4
        |-IntegerLiteral <line:14:23> 'int' 2
        `-CXXConstructExpr <line:16:5> 'std::optional<int>' 'void () noexcept' list

But when we have {} we end up with:

      `-InitListExpr <col:9, line:16:5> 'options'
        |-IntegerLiteral <line:13:23> 'int' 4
        |-IntegerLiteral <line:14:22> 'int' 2
        `-CXXDefaultInitExpr <line:16:5> 'std::optional<int>'

Maybe fields that are default constructable and user provided constructor should be excluded, or all fields should always be required to be initialized in designated initializer, whatever they have in-class initializer or not.

[vvd170501]

-Wmissing-designated-field-initializers is silenced by the default initializer for both classes and non-class fields, but I didn't change this behavior. -Wmissing-field-initializers could already be silenced by {} before my PR: #56628 (comment)

all fields should always be required to be initialized in designated initializer, whatever they have in-class initializer or not

This is the behavior of -Wmissing-field-initializers since 9108897 and it matches with GCC (example: https://godbolt.org/z/b9WaGnTh4), so I guess it shouldn't be changed.

[5chmidti]

I think that the check and the warning are correct in what they diagnose. The check detects two kinds of issues. Unnecessary initializers in constructors, and unnecessary initializers at the member-declaration. I think it would make sense to allow to configure what the check should detect. For the provided example, you'd disable the checking for redundant field initializations, while keeping the redundant constructor field initialization diagnostics.

Also note the example in #119094

[nico]

For what it's worth, gcc's -Wmissing-field-initializers also flags this. So changing clangd seems better than changing clang at this point.