Implementing missing-braces warning

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

Implementing missing-braces warning

Tanya Lattner
Hello,

I'm attempting to implement the missing-braces warning.

With this example:
int a[2][2] = { 0, 1, 2, 3 };

It should generate a warning because its missing braces around the first two values, and the last 2 (int a[2][2] = { { 0, 1 }, { 2, 3 } }).

gcc outputs the warning like this:
/tmp/warn-missing-braces.c:3: warning: missing braces around initializer
/tmp/warn-missing-braces.c:3: warning: (near initialization for ‘a[0]’)

So I have two questions.

When I output this warning, what should it look like? Should code modification hints be given? Is one warning output or one for each set of braces needed?

As for implementation details, in SemaInit::CheckSubElementType we assume brace elision. I assume that this is where I should output my warning, but when i do it there, I get one for each set (so 2 warnings). Any advice here?

Thanks,
Tanya



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Implementing missing-braces warning

Douglas Gregor

On Jan 27, 2010, at 3:44 PM, Tanya Lattner wrote:

> Hello,
>
> I'm attempting to implement the missing-braces warning.
>
> With this example:
> int a[2][2] = { 0, 1, 2, 3 };
>
> It should generate a warning because its missing braces around the first two values, and the last 2 (int a[2][2] = { { 0, 1 }, { 2, 3 } }).
>
> gcc outputs the warning like this:
> /tmp/warn-missing-braces.c:3: warning: missing braces around initializer
> /tmp/warn-missing-braces.c:3: warning: (near initialization for ‘a[0]’)
>
> So I have two questions.
>
> When I output this warning, what should it look like?

I think GCC has the warning mostly right. Pointing out which subobject we're in ('a[0]') is really helpful, although it should be folded into the diagnostic itself. Clang should also highlight (with a source range) all of the initializers that are conceptually initializing that subobject, e.g.,

warning: missing braces around initialization of subobject 'a[0]'
int a[2][2] = { 0, 1, 2, 3 };
                ^~~~
                {   }

I'm not thrilled with the use of the word "missing", which seems to imply an actual error. I somewhat prefer:

        warning: suggest braces around initialization of subobject 'a[0]'

> Should code modification hints be given?

Yes! The hints should definitely insert the '{' and '}' in the right place.

> Is one warning output or one for each set of braces needed?

One for each set of braces, IMO.

> As for implementation details, in SemaInit::CheckSubElementType we assume brace elision. I assume that this is where I should output my warning, but when i do it there, I get one for each set (so 2 warnings). Any advice here?


Did you try CheckImplicitInitList? IIRC, that's where we're implicitly stepping into a subobject when initializing a list.

        - Doug
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Implementing missing-braces warning

Tanya Lattner

>
> I think GCC has the warning mostly right. Pointing out which subobject we're in ('a[0]') is really helpful, although it should be folded into the diagnostic itself. Clang should also highlight (with a source range) all of the initializers that are conceptually initializing that subobject, e.g.,
>
> warning: missing braces around initialization of subobject 'a[0]'
> int a[2][2] = { 0, 1, 2, 3 };
>                ^~~~
>                {   }
>
> I'm not thrilled with the use of the word "missing", which seems to imply an actual error. I somewhat prefer:
>
> warning: suggest braces around initialization of subobject 'a[0]'
>

Ok. So, how do I get "a[0]" from the InitListExpr?

>> Should code modification hints be given?
>
> Yes! The hints should definitely insert the '{' and '}' in the right place.
>

Ok, did this.

>> Is one warning output or one for each set of braces needed?
>
> One for each set of braces, IMO.

Ok, did this too.

>
>> As for implementation details, in SemaInit::CheckSubElementType we assume brace elision. I assume that this is where I should output my warning, but when i do it there, I get one for each set (so 2 warnings). Any advice here?
>
>
> Did you try CheckImplicitInitList? IIRC, that's where we're implicitly stepping into a subobject when initializing a list.
>

Awesome. This works great!

Thanks Doug!

-Tanya


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: Implementing missing-braces warning

Douglas Gregor

On Jan 29, 2010, at 1:58 PM, Tanya Lattner wrote:

>
>>
>> I think GCC has the warning mostly right. Pointing out which subobject we're in ('a[0]') is really helpful, although it should be folded into the diagnostic itself. Clang should also highlight (with a source range) all of the initializers that are conceptually initializing that subobject, e.g.,
>>
>> warning: missing braces around initialization of subobject 'a[0]'
>> int a[2][2] = { 0, 1, 2, 3 };
>>               ^~~~
>>               {   }
>>
>> I'm not thrilled with the use of the word "missing", which seems to imply an actual error. I somewhat prefer:
>>
>> warning: suggest braces around initialization of subobject 'a[0]'
>>
>
> Ok. So, how do I get "a[0]" from the InitListExpr?

You can't really get it from the InitListExpr. However, the InitializedEntity has this information through the entity kinds (member, array element, and vector element are the only ones that matter, until it terminates at the variable) if you start at the current entity and walk the parent list, you could construct  the subobject path correctly even deeply-nested cases.

I think this would be great, and could be used to provide great diagnostics for any failed initialization within an initializer list, but that might be more work than you meant to sign up for. Alternatively, you could reword the diagnostic to avoid having to construct that path, e.g.,

        warning: suggest braces around initialization of subobject of type 'int[2]'

>>> Should code modification hints be given?
>>
>> Yes! The hints should definitely insert the '{' and '}' in the right place.
>>
>
> Ok, did this.
>
>>> Is one warning output or one for each set of braces needed?
>>
>> One for each set of braces, IMO.
>
> Ok, did this too.

Cool.

        - Doug
_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev