public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Sebor <msebor@gmail.com>
To: Sandra Loosemore <sandra@codesourcery.com>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] document that alias and target must have the same type
Date: Fri, 14 Feb 2020 18:30:00 -0000	[thread overview]
Message-ID: <083a81f5-a658-ca44-fb0d-1b7659ca8f2d@gmail.com> (raw)
In-Reply-To: <85d28ca7-bc65-ce67-ba1f-60e30fc376b5@codesourcery.com>

[-- Attachment #1: Type: text/plain, Size: 4022 bytes --]

On 2/13/20 3:55 PM, Sandra Loosemore wrote:
> On 2/5/20 1:13 PM, Martin Sebor wrote:
> 
>> diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
>> index ec99c38a607..3634ce1c423 100644
>> --- a/gcc/doc/extend.texi
>> +++ b/gcc/doc/extend.texi
>> @@ -2557,8 +2557,11 @@ __attribute__ ((access (write_only, 1, 2), 
>> access (read_write, 3))) int fgets (c
>>
>>  @item alias ("@var{target}")
>>  @cindex @code{alias} function attribute
>> -The @code{alias} attribute causes the declaration to be emitted as an
>> -alias for another symbol, which must be specified.  For instance,
>> +The @code{alias} attribute causes the declaration to be emitted as an 
>> alias
>> +for another symbol, which must have been previously declared with the 
>> same
>> +type, and for variables same size and alignment.  Declaring an alias 
>> with
>> +a different type than the target is undefined and may be diagnosed.  For
>> +instance, an alias for another symbol, which must be specified.  For 
>> instance,
>>
>>  @smallexample
>>  void __f () @{ /* @r{Do something.} */; @}
> 
> This is kind of garbled.  Do you have an extraneous sentence beginning 
> "For instance" in there?  It doesn't flow from the text you added before 
> that.

That does look garbled.  I've fixed that in the revised patch.

> 
>> @@ -3919,31 +3922,41 @@ results in warning on line 5.
>>
>>  @item weak
>>  @cindex @code{weak} function attribute
>> -The @code{weak} attribute causes the declaration to be emitted as a weak
>> -symbol rather than a global.  This is primarily useful in defining
>> -library functions that can be overridden in user code, though it can
>> -also be used with non-function declarations.  Weak symbols are supported
>> -for ELF targets, and also for a.out targets when using the GNU assembler
>> -and linker.
>> +The @code{weak} attribute causes a declaration of an external symbol
>> +to be emitted as a weak symbol rather than a global.  This is primarily
>> +useful in defining library functions that can be overridden in user 
>> code,
>> +though it can also be used with non-function declarations.  The 
>> overriding
>> +symbol must have the same type, and for variables size, and alignment as
>> +the weak symbol.  Weak symbols are supported for ELF targets, and 
>> also for
>> +a.out targets when using the GNU assembler and linker.
> 
> The "for variables" clause is awkward and has a comma in the wrong 
> place.  I'd split this into a separate sentence, like
> 
> The overriding symbol must have the same type as the weak symbol.  If it 
> is a variable it must also have the same size and alignment.

Agreed, that looks better.

> 
>>  @item weakref
>>  @itemx weakref ("@var{target}")
>>  @cindex @code{weakref} function attribute
>>  The @code{weakref} attribute marks a declaration as a weak reference.
>>  Without arguments, it should be accompanied by an @code{alias} attribute
>> -naming the target symbol.  Optionally, the @var{target} may be given as
>> -an argument to @code{weakref} itself.  In either case, @code{weakref}
>> -implicitly marks the declaration as @code{weak}.  Without a
>> -@var{target}, given as an argument to @code{weakref} or to @code{alias},
>> -@code{weakref} is equivalent to @code{weak}.
>> +naming the target symbol.  Alernatively, @var{target} may be given as
> 
> s/Alernatively/Alternatively/
> 
>> +an argument to @code{weakref} itself, naming the target definition of
>> +the alias.  The the @var{target} must have the same type, and for
> 
> s/The the/The/
> 
>> +variables also the same size and alignment as the declaration.  In 
>> either
> 
> Same comments above re splitting this into two sentences to avoid 
> awkward wording.

I've spit it up into multiple sentences as you suggested.  Attached
is the updated revision that I plan to commit unless you (or anyone
else) have further suggestions.

Thanks for the review!

Martin

> 
> The rest of this patch looks OK.
> 
> -Sandra


[-- Attachment #2: gcc-doc-attr-alias.diff --]
[-- Type: text/x-patch, Size: 5172 bytes --]

gcc/ChangeLog:

	* doc/extend.texi (attribute alias): Mention type requirement
	(attribute weak): Same.
	(attribute weakref): Correct invalid example.

diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index 5739063b330..b7f462a76b0 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -2557,8 +2557,11 @@ __attribute__ ((access (write_only, 1, 2), access (read_write, 3))) int fgets (c
 
 @item alias ("@var{target}")
 @cindex @code{alias} function attribute
-The @code{alias} attribute causes the declaration to be emitted as an
-alias for another symbol, which must be specified.  For instance,
+The @code{alias} attribute causes the declaration to be emitted as an alias
+for another symbol, which must have been previously declared with the same
+type, and for variables, also the same size and alignment.  Declaring an alias
+with a different type than the target is undefined and may be diagnosed.  As
+an example, the following declarations:
 
 @smallexample
 void __f () @{ /* @r{Do something.} */; @}
@@ -2566,9 +2569,9 @@ void f () __attribute__ ((weak, alias ("__f")));
 @end smallexample
 
 @noindent
-defines @samp{f} to be a weak alias for @samp{__f}.  In C++, the
-mangled name for the target must be used.  It is an error if @samp{__f}
-is not defined in the same translation unit.
+define @samp{f} to be a weak alias for @samp{__f}.  In C++, the mangled name
+for the target must be used.  It is an error if @samp{__f} is not defined in
+the same translation unit.
 
 This attribute requires assembler and object file support,
 and may not be available on all targets.
@@ -3919,31 +3922,43 @@ results in warning on line 5.
 
 @item weak
 @cindex @code{weak} function attribute
-The @code{weak} attribute causes the declaration to be emitted as a weak
-symbol rather than a global.  This is primarily useful in defining
-library functions that can be overridden in user code, though it can
-also be used with non-function declarations.  Weak symbols are supported
-for ELF targets, and also for a.out targets when using the GNU assembler
-and linker.
+The @code{weak} attribute causes a declaration of an external symbol
+to be emitted as a weak symbol rather than a global.  This is primarily
+useful in defining library functions that can be overridden in user code,
+though it can also be used with non-function declarations.  The overriding
+symbol must have the same type as the weak symbol.  In addition, if it
+designates a variable it must also have the same size and alignment as
+the weak symbol.  Weak symbols are supported for ELF targets, and also
+for a.out targets when using the GNU assembler and linker.
 
 @item weakref
 @itemx weakref ("@var{target}")
 @cindex @code{weakref} function attribute
 The @code{weakref} attribute marks a declaration as a weak reference.
 Without arguments, it should be accompanied by an @code{alias} attribute
-naming the target symbol.  Optionally, the @var{target} may be given as
-an argument to @code{weakref} itself.  In either case, @code{weakref}
-implicitly marks the declaration as @code{weak}.  Without a
-@var{target}, given as an argument to @code{weakref} or to @code{alias},
-@code{weakref} is equivalent to @code{weak}.
+naming the target symbol.  Alternatively, @var{target} may be given as
+an argument to @code{weakref} itself, naming the target definition of
+the alias.  The @var{target} must have the same type as the declaration.
+In addition, if it designates a variable it must also have the same size
+and alignment as the declaration.  In either form of the declaration
+@code{weakref} implicitly marks the declared symbol as @code{weak}.  Without
+a @var{target} given as an argument to @code{weakref} or to @code{alias},
+@code{weakref} is equivalent to @code{weak} (in that case the declaration
+may be @code{extern}).
 
 @smallexample
-static int x() __attribute__ ((weakref ("y")));
+/* Given the declaration: */
+extern int y (void);
+
+/* the following... */
+static int x (void) __attribute__ ((weakref ("y")));
+
 /* is equivalent to... */
-static int x() __attribute__ ((weak, weakref, alias ("y")));
-/* and to... */
-static int x() __attribute__ ((weakref));
-static int x() __attribute__ ((alias ("y")));
+static int x (void) __attribute__ ((weakref, alias ("y")));
+
+/* or, alternatively, to... */
+static int x (void) __attribute__ ((weakref));
+static int x (void) __attribute__ ((alias ("y")));
 @end smallexample
 
 A weak reference is an alias that does not by itself require a
@@ -3956,10 +3971,10 @@ symbol, not necessarily in the same translation unit.
 The effect is equivalent to moving all references to the alias to a
 separate translation unit, renaming the alias to the aliased symbol,
 declaring it as weak, compiling the two separate translation units and
-performing a link with relocatable output (ie: @code{ld -r}) on them.
+performing a link with relocatable output (i.e.@: @code{ld -r}) on them.
 
-At present, a declaration to which @code{weakref} is attached can
-only be @code{static}.
+A declaration to which @code{weakref} is attached and that is associated
+with a named @code{target} must be @code{static}.
 
 @end table
 

  reply	other threads:[~2020-02-14 18:30 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-05  1:05 Martin Sebor
2020-02-05 20:13 ` Martin Sebor
2020-02-12 21:02   ` [PING PATCH] " Martin Sebor
2020-02-13 22:55   ` [PATCH] " Sandra Loosemore
2020-02-14 18:30     ` Martin Sebor [this message]
2020-02-15  0:08       ` Sandra Loosemore
2020-02-13 22:45 ` Sandra Loosemore

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=083a81f5-a658-ca44-fb0d-1b7659ca8f2d@gmail.com \
    --to=msebor@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=sandra@codesourcery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).