incubator-stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: [PATCH] STDCXX-563
Date Thu, 21 Feb 2008 05:31:05 GMT
Scott Zhong wrote:
> Split _mutex.h into
> 
> _mutex-aix.h
> _mutex-deccxx.h
> _mutex.h
> _mutex-i386gcc.h
> _mutex-ia64-x86-64.h
> _mutex-parisc.h
> _mutex-sgi.h
> _mutex-sparc.h
> 
> http://www.fileden.com/files/2008/1/30/1729926/patch.stdcxx-563.tar
> 

Clearing the decks before my trip next week, I'm finally looking
at this patch...

My first concern stems from the names of the new files: it seems
they reflect the partitioning of the current _mutex.h, which is
in some cases done for compilers and the intrinsics they provide,
and in other cases for hardware architectures and/or operating
systems. I hadn't realized that this was done so inconsistently
and was hoping for a cleaner split. I don't think we should have
_mutex-sgi.h right along _mutex-sparc.h, and certainly not
_mutex-i386gcc.h. We could achieve the same effect while at the
same time employing a more cohesive source organization under
a consistent naming convention by splitting the file by, say, the
OS, or the hardware architecture, or both, and including one from
the other as needed.

I would expect _mutex.h to be a a thin "dispatch" kind of a header
with the only task to #include the header appropriate for the
platform (either compiler, or OS, or hardware architecture, but
not a mixture of all three). Such a header wouldn't contain any
platform-specific code (not even the Windows intrinsics).

My other concern has to do with the very top of some of the new
files, such as _mutex-aix.h:

     }   // namespace __rw

     #include <sys/atomic_op.h>

     _RWSTD_NAMESPACE (__rw) {

First, the guard is missing, but it also looks like the opening
brace must be in _mutex.h? I don't think this is allowed by the
language, but even if it was, we wouldn't want to do that. It's
just not how source files should be structured. See, for example
http://accu.org/index.php/journals/473

I suggest you look at how the rw/_config.h header has been split
up (http://svn.apache.org/viewvc?view=rev&revision=382600) and
model the same approach in _mutex.h. If splitting it by compiler
doesn't make sense, maybe doing it by OS will. Or by hardware
architecture. Or even all three (although I'd try hard to keep
the number of new files to some reasonable minimum).

Martin

Mime
View raw message