| Summary: | All the WebSocket tests crash | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Adam Barth <abarth> | ||||
| Component: | New Bugs | Assignee: | Adam Barth <abarth> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | abarth, andersca, ap, darin, ukai, zecke | ||||
| Priority: | P2 | ||||||
| Version: | 528+ (Nightly build) | ||||||
| Hardware: | Other | ||||||
| OS: | OS X 10.5 | ||||||
| Attachments: |
|
||||||
|
Description
Adam Barth
2010-10-03 01:03:50 PDT
Created attachment 69583 [details]
Patch
Comment on attachment 69583 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=69583&action=review You could keep the 'const'. The patch looks sane. > WebCore/ChangeLog:9 > + That doesn't work in C++. Instead, we need to actually store the You could remove the extra space. Committed r68984: <http://trac.webkit.org/changeset/68984> The C++ standard, section 12.5, paragraph 5, says: "The temporary to which the reference is bound [...] persists for the lifetime of the reference except as specified below.” And the text below does not list any exception which applies here. So the temporary objects here will last the lifetime of the reference. So this code change should not have had any effect at all. This is a fairly common idiom in WebCore code. People have submitted patches to "fix" such code several times in the past, but reviewers stopped that before, and the problem always turned out to be something different. When did this start? Did this happen on buildbots? (In reply to comment #5) > This is a fairly common idiom in WebCore code. People have submitted patches to "fix" such code several times in the past, but reviewers stopped that before, and the problem always turned out to be something different. I was not aware that it is valid until the end of the block. I was doing this example http://paste.lisp.org/display/115156 for adam to show that it is at least valid for function calls. The other thing is, I think it is to be preferred to not use const reference as it can have suprising effects and as long as the function being called is inline, gcc will be able to eliminate the copy (since early gcc 3.x), as can be seen when compiling/executing the example. Anyway, I will be more careful. > > When did this start? Did this happen on buildbots? Leaving that to Adam. |