Bug 24535

Summary:

Missing line terminator chars (;)

Product:

WebKit

Reporter:

Norbert Leser <norbert.leser>

Component:

WebCore JavaScript

Assignee:

David Levin <levin>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, hausmann, mrowe

Priority:

Version:

528+ (Nightly build)

Hardware:

All

OS:

All

Bug Depends on:

Bug Blocks:

27065

Attachments:

Description	Flags
Proposed fix for bug 24535	none
Proposed fix for 24535	darin: review+

Norbert Leser

Reported 2009-03-11 19:05:03 PDT

Various statements with macro calls, including in generated files, are missing missing line terminator character (;). This is ignored by most compilers but symbian compilers fail. Patch will follow.

Attachments
Proposed fix for bug 24535 (9.55 KB, patch) 2009-03-11 19:07 PDT, Norbert Leser	no flags	Details Formatted Diff Diff
Proposed fix for 24535 (10.13 KB, patch) 2009-03-13 10:15 PDT, Norbert Leser	darin: review+	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Norbert Leser

Comment 1 2009-03-11 19:07:29 PDT

Created attachment 28510 [details] Proposed fix for bug 24535

Mark Rowe (bdash)

Comment 2 2009-03-11 19:52:17 PDT

These macros expand to code that already include the trailing semicolons. Why is this change needed?

Alexey Proskuryakov

Comment 3 2009-03-12 00:02:24 PDT

If we do this, we should arguably remove semicolons from macro definitions.

Norbert Leser

Comment 4 2009-03-12 08:02:07 PDT

(In reply to comment #3) > If we do this, we should arguably remove semicolons from macro definitions. > I agree with your proposal - if it's just for the sake of consistency and cleanliness of the code. In fact, as far as I can tell, the COMPILE_ASSERT macro in Assert.h is the only one that has an implicit ";" It is common practice anyway to not have a trailing ";" in #define's I'd be glad to update the patch with this change in Assert.h, if there's an agreement.

Alexey Proskuryakov

Comment 5 2009-03-12 08:26:35 PDT

I think that would be nice as a cleanup patch, yes. It's extremely surprising that any compiler would choke on the current code though.

Norbert Leser

Comment 6 2009-03-13 10:15:59 PDT

Created attachment 28580 [details] Proposed fix for 24535 I added Assertions.h (removal of trailing ";" from COMPILE_ASSERT macro definition) to this updated patch, as discussed. Otherwise, unchanged to initial patch.

Darin Adler

Comment 7 2009-03-13 10:18:23 PDT

Comment on attachment 28580 [details] Proposed fix for 24535 Seems OK, r=me

David Levin

Comment 8 2009-03-20 15:28:22 PDT

Assigned to levin for landing. Please add a link to the bug in the ChangeLog in the future and there is no need to leave this "WARNING: NO TEST CASES ADDED OR CHANGED" in the ChangeLog.

David Levin

Comment 9 2009-03-20 16:56:37 PDT

Committed as r41879.

Eric Seidel (no email)

Comment 10 2009-05-04 01:58:16 PDT

Comment on attachment 28510 [details] Proposed fix for bug 24535 A later version of this was reviewed already. Clearing review flag to remove from queue.

Note You need to log in before you can comment on or make changes to this bug.