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: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Bug Depends on: | |||||||||
Bug Blocks: | 27065 | ||||||||
Attachments: |
|
Description
Norbert Leser
2009-03-11 19:05:03 PDT
Created attachment 28510 [details] Proposed fix for bug 24535 These macros expand to code that already include the trailing semicolons. Why is this change needed? If we do this, we should arguably remove semicolons from macro definitions. (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. 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. 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.
Comment on attachment 28580 [details]
Proposed fix for 24535
Seems OK, r=me
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. 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. |