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: [jira] Commented: (STDCXX-563) split up rw/_mutex.h
Date Thu, 27 Mar 2008 20:50:09 GMT
Travis Vitek wrote:
>>
>>
>> Martin Sebor wrote:
>>
>> Here are some observations and suggestions regarding the patch:
>>
>> # Underscores separating components of file names should be replaced with
>>  dashes for consistency with the {{rw/_config-*.h}} headers.
>> # There's a typo in the name of {{_atomic_aplha.h}}. I suspect the name
>>  should be changed to {{_atomic-deccxx.h}} since the primitives seem
>>  specific to the compiler, not to the hardware architecture.
>> # What is {{_atomic_generic.h}} for and shouldn't it be merged with
>>  {{_atomic.h}}?
>> # What compilers is {{_atomic_ia64_x64.h}} used by? If all of them on
>>  IA64 as well as x86_64 (in LLP64), maybe it should be called
>>  {{_atomic-x64.h}}. I see a lof of #ifdefs for MSVC. Would it make
>>  sense to split it up into {{_atomic-msvc.h}} and whatever else?
>> # I believe {{_atomic_mips.h}} is specific to the MIPSpro compiler and
>>  couldn't be used with gcc on the MIPS architecture. It should be
>>  renamed to {{_atomic_mipspro.h}}
>> # I'm not quite sure what to do with {{_atomic_mutex.h}}. Ideally, we
>>  would have atomic operations everywhere. If there is a platform where
>>  we (sometimes) need to use the mutex version (I think you mentioned
>>  PA-RISC) I guess we need to keep it but it doesn't make me very happy...
>> # If {{_atomic_powerpc.h}} is specific to IBM XLC++ (and can't be used
>>  by gcc) it should be renamed to {{_atomic-xlc.h}}.
>> # Would {{_mutex-win32.h}} be a better name than {{_mutex-windows.h}}?
>>
> 
> So, just to be clear, it seems that you're proposing a set of headers
> for the compiler and another set for the thread lib. i.e. we would have
> rw/_config-<compiler>.h and rw/_config-<thread lib>.h but no config files
> for the architecture or OS.

Not necessarily. Whatever makes sense. If we have a set of atomics
that are unique to, say, SPARC, I think it would be perfectly fine
to have _atomic-sparc.h. But if the same set of functions works with,
say, Sun C++ (because they are exposed as compiler intrinsics as they
often tend to be), regardless of the hardware, then it makes little
sense (to me) to call the header _atomic-sparc.h or _atomic-x86.h.
Rather, I would expect the header to be called _atomic-sunpro.h.
If, for example, Solaris happens to expose its own atomic API that
we wanted to use with compilers other than Sun C++ we could also
have _atomic-sunos.h (or _atomic-solaris.h).

> 
> If so, I'm fine with that. I can imagine that Scott/Farid would have liked
> this to have been stated up front.

This is an iterative, collaborative effort. I didn't design how
the header should be split up. I'm just giving feedback on the
latest patch, just like you are doing. If the feedback we all
give precipitates changes to the patch, that's a good thing.
It makes the final product better. If no feedback ever led to
any changes there would be no point in providing it or in
reviewing code to begin with.

For the record, though, an outline of what I thought was a good
way to partition the function in the post below. It obviously
wasn't detailed enough. I should have perhaps spent more time
thinking about it before giving my comments. It could have saved
some time.
http://www.nabble.com/-PATCH--STDCXX-563-tt15442185.html#a15442185

> 
>> # One of {{_atomic-x86.h}} and the src/i86] directory should be renamed
>>  for consistency. It seems that the commonly used abbreviation used for
>>  the Intel 8086-derived processors (e.g., 80386, 80486) is x86 -- see
>>  the Wikipedia article.
> 
> nit.
> 
>> Finally, I wonder if instead of adding suffixes to these files and worry
>> about being consistent every time we add a new one it would make sense to
>> add platform-specific directories under {{include/rw/}} instead and move
>> the corresponding files (as well as the {{rw/_config-*.h}} headers) there.
>> Thoughts?
>>
> 
> I don't know. If we are already doing it one way, we should either do it
> the same way, or we should go back and change the other arrangement to
> match.

Right. IMO, we should go with whichever of the approaches is better.
Which one in your opinion is it?

Martin

Mime
View raw message