Bug 47055 – All the WebSocket tests crash

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adam Barth]

All the WebSocket tests crash

[Adam Barth]

Created attachment 69583 [details]
Patch

[Holger Freyther]

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.

[Adam Barth]

Committed r68984: <http://trac.webkit.org/changeset/68984>

[Darin Adler]

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.

[Alexey Proskuryakov]

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?

[Holger Freyther]

(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.