diff mbox

[DOC] symbol rename pragmas

Message ID 5371FEE6.8010702@LimeGreenSocks.com
State New
Headers show

Commit Message

David Wohlferd May 13, 2014, 11:15 a.m. UTC
Thank you for taking the time to review this.

>> the current text makes reference to "compatibility with the Solaris system
>> headers," the remaining pragma is (according to the existing text)
>> "currently on all platforms."  This makes referring to Solaris both
>> superfluous and potentially confusing.
> I disagree: #pragma redefine_extname does exist for Solaris
> compatibility only, even if it now works on all platforms.  So please
> continue to state so.

Hmm.  Looking at the original text:

"For compatibility with the Solaris system headers, GCC supports two 
#pragma directives that change the name used in assembly for a given 
declaration."

As I read that, I only saw the motivation for why this pragma was 
added.  Even now I don't read this as *restricting* these pragmas to 
Solaris.  Especially when you combine that with:

- The text below which states that redefine_extname is "currently on all 
platforms."
- This pragma isn't under the "Solaris Pragmas" headings, but under its 
own (apparently platform-neutral) heading.
- It does in fact work on other platforms.  Even the 
__PRAGMA_REDEFINE_EXTNAME macro is there.

Considering all these points, someone could easily conclude this was not 
Solaris-specific.  While that may not have been the intent when this 
text was written, that's how I read it, and others may have too.  Are 
you sure you want me to make this change?

I'm not trying to pick a fight here.  I'm not using the pragma, so this 
doesn't affect me.  It's just that this has been out there for a long 
time, affects platforms other than Solaris, and I'm not immediately 
clear on what problems supporting other platforms creates.||

In the end I guess I'd just like to be sure some other OS Port 
Maintainer (or some distraught user) isn't going to be emailing me 
demanding to know why "I" dropped support for this from their platform.

That said.

If this does need to change, I'd recommend something like the attached.  
Note that since the previous patch was checked in, this patch updates 
from that point.  It deletes the "Symbol-Renaming Pragmas" node, and 
moves (most of) the redefine_extname text into the "Solaris Pragmas" node.

If reading the patch doesn't make it clear what I've done, you can see 
the end result here: http://www.LimeGreenSocks.com/gcc/Solaris-Pragmas.html

I believe this makes it crystal clear that the only platform where gcc 
supports this pragma is Solaris.  If you approve (and I don't hear from 
some other Port Maintainer or distraught user), I'll submit this as a 
new patch.  Be aware, I plan to put your name in the changelog.

dw

Comments

Rainer Orth May 13, 2014, 11:36 a.m. UTC | #1
Hi David,

> Thank you for taking the time to review this.
>
>>> the current text makes reference to "compatibility with the Solaris system
>>> headers," the remaining pragma is (according to the existing text)
>>> "currently on all platforms."  This makes referring to Solaris both
>>> superfluous and potentially confusing.
>> I disagree: #pragma redefine_extname does exist for Solaris
>> compatibility only, even if it now works on all platforms.  So please
>> continue to state so.
>
> Hmm.  Looking at the original text:
>
> "For compatibility with the Solaris system headers, GCC supports two
> #pragma directives that change the name used in assembly for a given
> declaration."
>
> As I read that, I only saw the motivation for why this pragma was added.

right, and that's why I want to keep this info.  If it weren't for
Solaris compatibility, this pragma wouldn't exist, and given that
heritage, I don't want to encourage its use elsewhere, even though it
does work.

> Even now I don't read this as *restricting* these pragmas to Solaris.
> Especially when you combine that with:
>
> - The text below which states that redefine_extname is "currently on all
> platforms."
> - This pragma isn't under the "Solaris Pragmas" headings, but under its own
> (apparently platform-neutral) heading.
> - It does in fact work on other platforms.  Even the
> __PRAGMA_REDEFINE_EXTNAME macro is there.

I've never claimed otherwise.

> Considering all these points, someone could easily conclude this was not
> Solaris-specific.  While that may not have been the intent when this text
> was written, that's how I read it, and others may have too.  Are you sure
> you want me to make this change?

I am, for the reasons stated above.

> I'm not trying to pick a fight here.  I'm not using the pragma, so this
> doesn't affect me.  It's just that this has been out there for a long time,
> affects platforms other than Solaris, and I'm not immediately clear on what
> problems supporting other platforms creates.||
>
> In the end I guess I'd just like to be sure some other OS Port Maintainer
> (or some distraught user) isn't going to be emailing me demanding to know
> why "I" dropped support for this from their platform.
>
> That said.
>
> If this does need to change, I'd recommend something like the attached.
> Note that since the previous patch was checked in, this patch updates from
> that point.  It deletes the "Symbol-Renaming Pragmas" node, and moves (most
> of) the redefine_extname text into the "Solaris Pragmas" node.
>
> If reading the patch doesn't make it clear what I've done, you can see the
> end result here: http://www.LimeGreenSocks.com/gcc/Solaris-Pragmas.html
>
> I believe this makes it crystal clear that the only platform where gcc
> supports this pragma is Solaris.  If you approve (and I don't hear from
> some other Port Maintainer or distraught user), I'll submit this as a new
> patch.  Be aware, I plan to put your name in the changelog.

You misunderstood completely: I'm not at all demanding to remove support
for that pragma in the cross-platform part of the docs or even in the
code, just ask to keep documenting the heritage so it's clear why it
exists and when to use it (or not).

Just restoring the first sentence you deleted describing the heritage
should be enough to do so.

	Rainer
Joseph Myers May 13, 2014, 5:41 p.m. UTC | #2
On Tue, 13 May 2014, Rainer Orth wrote:

> right, and that's why I want to keep this info.  If it weren't for
> Solaris compatibility, this pragma wouldn't exist, and given that
> heritage, I don't want to encourage its use elsewhere, even though it
> does work.

I can see definite cases where it could be useful in glibc headers, to 
redirect functions to the appropriate version for a particular standard in 
the implicitly preincluded stdc-predef.h even if the header declaring the 
function in question isn't included (ISO C and POSIX allow you to declare 
standard functions yourself if the standard prototype for the function 
doesn't involve any type defined in a header).  So, I think it's quite 
right for it to be available on all platforms as a feature that can help 
in implementing certain standard requirements.
David Wohlferd May 13, 2014, 10:37 p.m. UTC | #3
> You misunderstood completely: I'm not at all demanding to remove support
> for that pragma in the cross-platform part of the docs or even in the
> code,

Whew!  Ok, that's good.  Sorry I went all crazy on you.  When I thought 
that *was* what you were asking for, I wanted to try to talk you out of it.

> just ask to keep documenting the heritage so it's clear why it
> exists and when to use it (or not).

Ok, I get this.  I've written text that "discouraged" using things in 
the past, even though technically they work just fine.

> Just restoring the first sentence you deleted describing the heritage
> should be enough to do so.

Speaking for myself, I didn't see the old text as favoring one course of 
action over another.  How would you feel about:

GCC supports a @code{#pragma} directive that changes the name used in
assembly for a given declaration. While this pragma is supported on all
platforms, it is intended primarily to provide compatibility with the
Solaris system headers. This effect can also be achieved using the asm
labels extension (@pxref{Asm Labels}).

To me, this subtly discourages using the pragma other than for Solaris, 
maintains the heritage, and immediately provides a viable alternative 
for other platforms.

dw
diff mbox

Patch

Index: extend.texi
===================================================================
--- extend.texi	(revision 210355)
+++ extend.texi	(working copy)
@@ -16657,7 +16657,6 @@ 
 * RS/6000 and PowerPC Pragmas::
 * Darwin Pragmas::
 * Solaris Pragmas::
-* Symbol-Renaming Pragmas::
 * Structure-Packing Pragmas::
 * Weak Pragmas::
 * Diagnostic Pragmas::
@@ -16851,8 +16850,7 @@ 
 @node Solaris Pragmas
 @subsection Solaris Pragmas
 
-The Solaris target supports @code{#pragma redefine_extname}
-(@pxref{Symbol-Renaming Pragmas}).  It also supports additional
+The Solaris target supports additional
 @code{#pragma} directives for compatibility with the system compiler.
 
 @table @code
@@ -16880,23 +16878,13 @@ 
 initialization (before @code{main}) or during shared module loading, by
 adding a call to the @code{.init} section.
 
-@end table
-
-@node Symbol-Renaming Pragmas
-@subsection Symbol-Renaming Pragmas
-
-GCC supports a @code{#pragma} directive that changes the name used in
-assembly for a given declaration. This effect can also be achieved
-using the asm labels extension (@pxref{Asm Labels}).
-
-@table @code
 @item redefine_extname @var{oldname} @var{newname}
 @cindex pragma, redefine_extname
 
 This pragma gives the C function @var{oldname} the assembly symbol
 @var{newname}.  The preprocessor macro @code{__PRAGMA_REDEFINE_EXTNAME}
-is defined if this pragma is available (currently on all platforms).
-@end table
+is defined if this pragma is available.  This effect can also be achieved
+using the asm labels extension (@pxref{Asm Labels}).
 
 This pragma and the asm labels extension interact in a complicated
 manner.  Here are some corner cases you may want to be aware of:
@@ -16919,6 +16907,8 @@ 
 always the C-language name.
 @end enumerate
 
+@end table
+
 @node Structure-Packing Pragmas
 @subsection Structure-Packing Pragmas