Bug 62097 – regression(87771): ternary operator incorrectly identified as bitfield

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Alex Milowski]

After the patch for bug 54807 was applied, the style check now incorrectly identifies code like:

   int heightDiff = m_scripts ? (m_scripts->offsetHeight() - maxHeight) / 2 : 0;

as a bitfield.  I had to change the code to:


   int heightDiff = m_scripts ?
                (m_scripts->offsetHeight() - maxHeight) / 2 
                : 0;

to get it to pass the style check.

[Eric Seidel (no email)]

I remember seeing that change go by.  Guess it needs some tweaking.

[Eric Seidel (no email)]

I wouldn't bother changing your code to make it pass the style check.  the style checker just needs to be fixed.

[Yong Li]

(In reply to comment #2)
> I wouldn't bother changing your code to make it pass the style check.  the style checker just needs to be fixed.

I should have filtered out the ?: case...

[Yong Li]

I'm working on this now.

[Yong Li]

Created attachment 96096 [details]
the patch

[Eric Seidel (no email)]

Comment on attachment 96096 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96096&action=review

> Tools/Scripts/webkitpy/style/checkers/cpp.py:2925
> +    matched = re.match(r'\s*((const|mutable)\s+)?(char|(short(\s+int)?)|int|long(\s+(long|int))?)\s+[a-zA-Z_][a-zA-Z0-9_]*\s*:\s*\d+\s*;', line)

seems you meant + not *, or?

Unfortunate that we don't have a nice constant for "identifier" since that long regexp you added may be used in other places?

[Yong Li]

Comment on attachment 96096 [details]
the patch

View in context: https://bugs.webkit.org/attachment.cgi?id=96096&action=review

>> Tools/Scripts/webkitpy/style/checkers/cpp.py:2925
>> +    matched = re.match(r'\s*((const|mutable)\s+)?(char|(short(\s+int)?)|int|long(\s+(long|int))?)\s+[a-zA-Z_][a-zA-Z0-9_]*\s*:\s*\d+\s*;', line)
> 
> seems you meant + not *, or?
> 
> Unfortunate that we don't have a nice constant for "identifier" since that long regexp you added may be used in other places?

The first character cannot be digit, so there must be one [a-zA-Z_]. Then the left can be 0-multiple [a-zA-Z0-9_]. But you are right, [a-zA-Z0-9_]+ should be enough in this case, and it is used at line 2791. I'm not sure if we prefer strict match or loose (for parsing speed?). Seems every places in the file are doing their own jobs. I tend to leave the refine to the future patches :-P

[WebKit Review Bot]

Comment on attachment 96096 [details]
the patch

Clearing flags on attachment: 96096

Committed r88179: <http://trac.webkit.org/changeset/88179>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.