RFR: 8058255: Native jbyte Atomic::cmpxchg for supported x86 platforms

[Kim Barrett]

On Oct 21, 2014, at 11:55 AM, Erik Österlund <erik.osterlund at lnu.se> wrote:
> 
> Bug:
> https://bugs.openjdk.java.net/browse/JDK-8058255
> 
> Webrev for the simple fix (which I intend to push initially):
> http://cr.openjdk.java.net/~jwilhelm/8058255/webrev.03.macro/
> 
> Webrev for the inheritance/template solution (which will be pushed after and is the intended "end" result):
> http://cr.openjdk.java.net/~jwilhelm/8058255/webrev.03.templates/
> 
> Webrev for the delta from the macro solution to the inheritance/template solution:
> http://cr.openjdk.java.net/~jwilhelm/8058255/webrev.03.incremental/

I haven't had time to do a real review, but I noted a couple of things
here.

(1) I really dislike the file name "templateIdioms.hpp".  It says
little or nothing about what is presently in that file, and with a
name like that it seems all to likely to become a dumping ground.  My
own preference is for relatively fine grained header files, so that I
can include only what I actually need in a given place.  So I'd prefer
something like is_base_of.hpp for that class, and not sure whether the
base_class struct also goes there or in its own file.  (Fine-grained
would argue the former.)

(2) It's been a while since I looked at it, but my recollection is
that the Boost implementation of is_base_of is substantially more
complex than what is presently in this file.  I don't know how much of
that (possible? assuming my recollection is correct) additional
complexity might be to correctly support edge cases that might not be
properly handled by this simple implementation (I wonder about CV
qualifiers and inaccessible or ambiguous base classes), and how much
was to deal with deficient compilers.  Related to that, I'd like to
seem some tests.

(3) The documentation and protocol for these structs is weak.  For
example, I would make only the "value" member of is_base_of public,
with the rest private, and comment it as something like:
 is_base_of<T1, T2>::value is true iff T1 is a base class of T2

(4) C++11 is_base_of<T, T> is true for class types, and similarly for
Boost is_base_of (which attempts to match C++11).  The is_base_of
proposed here is false in that case.  Matching the "standard" behavior
would be preferable.  (I think the under review is_base_of differs
from C++11 for some combinations of CV qualifiers too, as C++11 says
the operation is performed without regard to CV qualifiers.)