[RFC] Relaxing coding standard rule for {}-initialization

Subprojects Flang
1

Current flang coding style requires the use of {}-initialization in all cases. The stated goal is to avoid errors related to unintended value conversion (specifically narrowing conversions that the {}-initialization prohibits).

Problem

The downside of that is that it makes the code less readable. Strict application of the rule leads to cases like

auto foo{[](Parameter p) {
  DoSomeWork(p);
}};

When the type is to be deduced (i.e. via auto), the narrowing conversion cannot happen, but we still need to write

auto foo{bar()};

While these cases may not seem problematic when presented on their own with lots of space above and below, they are harder to parse visually when surrounded by lines of code.

Compare with

auto foo = [](Parameter p) {
  DoSomeWork(p);
};

or

auto foo = bar();

Proposal (amended)

I propose that we limit the use of bracketed initialization to cases where

  • the initialized object is of arithmetic type and the type is explicitly specified, and
  • the initialized object is of aggregate type.

Proposal (original)

[This is only for historical record.]

I propose that we stop requiring the bracketed initialization. The narrowing conversion issue is almost never relevant in practice, most cases are simply initializations of local variables.

If that’s deemed too extreme I am open to some compromises. My objective is to make the code easier to read.

1 Like
2

I agree that lambda syntax is much more readable with =. Could we perhaps explicitly relax the rules for that case?

3

I agree that brace initialization is not generally very readable.

At the call today, it was mentioned that one of the motivations for requiring brace initialization was to avoid inadvertent narrowing. Was there also a concern about unnecessary copying when initializing aggregates? Or am I thinking of something else?

4

I think brace initialization makes sense for aggregates where it is an aggregate-initialization as opposed to the direct-initialization for non-aggregate types.

5

I think we still need to be careful about narrowing:

int x = wide();   // compiles, no warning under -Wall
int y{wide()};    // error: cannot be narrowed from 'long long' to 'int'

I’d rather not lose the warning.

Another point: if someone accidentally does auto x = {init}, this makes std::initializer_list, which was probably not intended.

1 Like
6

My argument is that this is less likely to happen by accident, since it requires more conscious attention from the person writing it. The cases that I think are risky are initializations of aggregates where I’d argue braced initialization makes more sense.

In cases where you actually want to initialize int y with a narrowed value, you have to write

int y{static_cast<int>(wide())};

which is even less readable.

7

Can we simply remove the mandate for using braced initialization? We can still “prefer” it for arithmetic types and for aggregates. For other types (like scoped enums, references, etc.) the narrowing conversion is not a thing, and there is no point in using it there.

1 Like
8

We still get narrowing checks for these cases (ll is long long):

  • const int &r{ll}
  • E e{300} (E is a scoped enum based on unsigned char)
  • Foo a{ll} (with Foo(int) constructor)

I don’t want to lose that.

After thinking about it some more, I would be ok with a blanket relaxation for auto.

1 Like
9

I think we need to concentrate on practical cases. If someone is actively trying to shoot themselves in the foot, braces won’t stop them.

10

The current restriction exists to prevent accidents. Issues haven’t come up precisely because the practice has been prohibited.

While the primary motivation for the change is readability in complex C++, that complexity is exactly where safety-first idioms are most necessary. Eugene’s concerns and proposed compromise make a lot of sense to me, and I support adopting that middle ground.

11

This. I actually had cases, where brace initialization was helpful to catch an issue. (I don’t remember exactly what they were, but at least one of them was 32/64 bit mismatch that could have caused a problem down the road.)

12

Or maybe because they couldn’t have happened.

You do realize that if we change 300 to 3 in Eugene’s example, the code will compile:

E e{300};  // error
E f{3};    // ok
E g = 3;   // error

Isn’t it ironic that the braces actually enable a clearly inappropriate code to compile?

I’m ok with braces for cases where they help, i.e. arithmetic types and aggregates. In other cases they only get in the way.

13

`What is the LLVM coding standard? Shouldn’t this be something aligned at the project level? Is this kind of bugs something that is more specific to Flang than other subprojects for some reasons?

14

The only thing I found is Do not use Braced Initializer Lists to Call a Constructor.

If you’re suggesting that flang should use LLVM coding standards instead of a patchwork of multiple ones, you have my full support, :slight_smile: