Closed Bug 573210 Opened 10 years ago Closed 10 years ago
Consistently qualify accesses to dependent base classes in xpcom C++ code
Per summary; clang (http://clang.llvm.org/) is more strict about lookup in dependent base classes than other C++ compilers. Patch attached. First patch in a series of patches to allow compiling Firefox with clang (see http://llvm.org/bugs/show_bug.cgi?id=5511).
Attachment #452444 - Flags: review?(benjamin)
Comment on attachment 452444 [details] [diff] [review] Patch This is ugly. If necessary, introduce a self_type typedef.
Attachment #452444 - Flags: review?(benjamin) → review-
I was trying to match local style, but if you prefer something more like this...
Comment on attachment 452476 [details] [diff] [review] Patch v2 dbaron, can you check this? It seems to me that clang is being incorrectly strict, since it can assume that identifiers which are not qualified with `typename` are dependent on the base type.
Attachment #452476 - Flags: review?(benjamin) → review?(dbaron)
cc'ing my compiler experts
This seems plausible based on rumors I've heard about name lookup in templates, but I really don't know much about that part of the C++ spec. Any idea if the gcc folks think it's a bug that they accept this code?
Why are you prefering base_type::method() rather than this->method() ? It seems like I'd default to preferring the latter unless you have a reason for the former.
I wasn't really thinking about it; I'll change it to this->method().
Updated to prefer "this->".
Comment on attachment 453300 [details] [diff] [review] Patch v3 r=dbaron Do you (still?) have commit access, or do you need someone to land this for you?
Attachment #453300 - Flags: review?(dbaron) → review+
(If you need/want someone to land it for you, you can add the 'checkin-needed' keyword.)
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.3
You need to log in before you can comment on or make changes to this bug.