Bug 118534 – [buildfix] Fix build on gcc 4.8.1 when compiling with -Werror=maybe-uninitialized

• Component: JavaScriptCore
• Status: NEW

[Hugo Parente Lima]

The warning is triggered because an uint32_t:2 is used to hold and Enum, so when this variable is used in a switch the compiler think anything could be there.

I'm aware that Efl builds with -Werror on, but those errors make me think if is better to compile with -Werror=no-maybe-uninitialized just to avoid patches like this one that doesn't help code wise and are here just make the compiler help.

[Hugo Parente Lima]

Created attachment 206399 [details]
Patch

[Chris Dumez]

cc'ing JSC people.

[Geoffrey Garen]

Comment on attachment 206399 [details]
Patch

I'd prefer to see "switch (static_cast<Enum>(info.mode))" here.

Adding the default case means the compiler won't warn us if we forget an enum value in the switch statement.

[Hugo Parente Lima]

(In reply to comment #3)
> (From update of attachment 206399 [details])
> I'd prefer to see "switch (static_cast<Enum>(info.mode))" here.
> 
> Adding the default case means the compiler won't warn us if we forget an enum value in the switch statement.

Good point, I thought about it but avoided due to the visual ugliness :-)

[Hugo Parente Lima]

(In reply to comment #3)
> (From update of attachment 206399 [details])
> I'd prefer to see "switch (static_cast<Enum>(info.mode))" here.
> 
> Adding the default case means the compiler won't warn us if we forget an enum value in the switch statement.

btw, the enum is annonymous.

struct ExpressionRangeInfo {
    enum {
        FatLineMode,
        FatColumnMode,
        FatLineAndColumnMode
    };
};

I'm not familiar enough with this code to suggest a name better than "FatMode" to this enum, so I would ask you a name for it.

Thanks.

[Geoffrey Garen]

> I'm not familiar enough with this code to suggest a name better than "FatMode" to this enum, so I would ask you a name for it.

I'd suggest "Mode". This struct is simple enough that a terse name is OK.

[Hugo Parente Lima]

(In reply to comment #6)
> > I'm not familiar enough with this code to suggest a name better than "FatMode" to this enum, so I would ask you a name for it.
> 
> I'd suggest "Mode". This struct is simple enough that a terse name is OK.

Even with the static cast g++ still blaming about:

'line' may be used uninitialized in this function [-Werror=maybe-uninitialized]

IMO better add -Werror=no-maybe-uninitialized to the build, this flag causes more headaches than advantages, until now it just generated small patches that decreased the code readability.