[webkit-reviews] review granted: [Bug 81061] Functions with C linkage should not return C++ classes : [Attachment 131758] the patch

[bugzilla-daemon at webkit.org]

Mark Rowe (bdash) <mrowe at apple.com> has granted Filip Pizlo
<fpizlo at apple.com>'s request for review:
Bug 81061: Functions with C linkage should not return C++ classes
https://bugs.webkit.org/show_bug.cgi?id=81061

Attachment 131758: the patch
https://bugs.webkit.org/attachment.cgi?id=131758&action=review

------- Additional Comments from Mark Rowe (bdash) <mrowe at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=131758&action=review


Looks fine to me.

> Source/JavaScriptCore/ChangeLog:3
> +	   Need a short description and bug URL (OOPS!)

Describe the problem!

> Source/JavaScriptCore/dfg/DFGOperations.h:160
> +// According to C++ rules, a type used for the return signature of function
with C linkage (i.e.
> +// 'extern "C"') needs to be POD in the C++-03 sense: loosely speaking it
must have nothing more than
> +// those members that would be valid in C. This means no constructors.

I don't think it's necessary to explain what a POD type is here. I'd simplify
this to something like:
// Since this type is used in the signature of a function declared with C
linkage it must be a POD type.

> Source/JavaScriptCore/llint/LLIntSlowPaths.h:44
> +// According to C++ rules, a type used for the return signature of function
with C linkage (i.e.
> +// 'extern "C"') needs to be POD in the C++-03 sense: loosely speaking it
must have nothing more than
> +// those members that would be valid in C. This means no constructors.

Same comment about the comment here.