492257 - Find classes with PRBool members

Status: RESOLVED INVALID

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Arpad Borsos [:Swatinem]]

Attachment: Class members that are PRBool (csv)

PRBool is 3 bytes larger than PRPackedbool and therefore shouldn't be used for class members.

This is a list of all those class members, generated using a newer version of my dehydra script from bug 492185.

Comment 1

[Benjamin Smedberg]

I don't think this is a consistent rule: I'm pretty sure there are stats which say that reading word-sized values improves performance compared to reading byte-sized values, so we should only be using PRPackedBool in cases where the memory overhead would be sizable.

Frankly though, I think we should switch to bool througout our codebase except for XPCOM interface definitions where the binary size matters.

Comment 2

[(dormant account)]

(In reply to comment #1)

> Frankly though, I think we should switch to bool througout our codebase except
> for XPCOM interface definitions where the binary size matters.

I agree. I'd say the safety benefits C++ bools is reason enough to switch to bool for internal interfaces.

As for finding prbools, I think it's more useful to have a 3rd column in analysis results in bug 492185 showing possible size reduction resulting in switching to "optimized" datatypes instead of a separate analysis.

Comment 3

[Brendan Eich [:brendan]]

There's another "ABI"-ish requirement to use JSBool instead of bool, for nanojit builtin function return type. See bug 453361.

/be

Comment 4

[Arpad Borsos [:Swatinem]]

There was a large rewrite some time ago to replace PRBool usage with c++ bools, so calling this INVALID.