Re: [jira] Commented: (STDCXX-563) split up rw/_mutex.h
Eric Lemings wrote: -Original Message- From: Martin Sebor [mailto:[EMAIL PROTECTED] On Behalf Of Martin Sebor Sent: Thursday, March 27, 2008 2:50 PM To: dev@stdcxx.apache.org Subject: Re: [jira] Commented: (STDCXX-563) split up rw/_mutex.h ... Right. IMO, we should go with whichever of the approaches is better. Which one in your opinion is it? I would prefer not using directories to organize configuration-specific files. If the configuration (cpu, os, compiler, whatever) is a part of the filename, the file can be placed in any arbitrary directory (not that it should be). So if these files shouldn't be placed in arbitrary directories (the only existing directory where could logically go right now is include/rw) what is the benefit of encoding the platform in their names? I'm not arguing for or against either approach (not yet, despite the list below :) just trying to understand the advantages. So far we have: Advantages of platform directories: * consistency: no risk of having individual files that deviate from a naming convention established by the directories * code organization: it's clear where to look for files specific to each platform * established practice: atomic sources under subdirectories of src/, e.g., src/i86 or src/ia64, etc. Disadvantages: ? Advantages of encoding platform in file names: * established practice: rw/_config-*.h headers, e.g., rw/_config-acc.h Disadvantages: * risk of naming inconsistencies when adding new files Martin If simpler, more generic filenames are needed, create links to the configuration-specific files. Brad.
RE: [jira] Commented: (STDCXX-563) split up rw/_mutex.h
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.
If so, I'm fine with that. I can imagine that Scott/Farid would have liked
this to have been stated up front.
# 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.
Travis
Re: [jira] Commented: (STDCXX-563) split up rw/_mutex.h
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
