On Thu, 8 Apr 2021, Martin Sebor via Gcc-patches wrote:
> There's another similar piece of code in pushdecl() that I didn't
> touch, although I couldn't come up with a test case showing it's
> necessary. Both hunks go back ages so I wonder if they might have
> been obviated by other improvements.
The other similar code in pushdecl is executed in cases where there are
multiple declarations of the same identifier in the same scope, e.g.:
int f (void);
void
g (void)
{
int f (void);
int f (void);
}
That particular example isn't interesting, but the idea is that the type
the declaration ends up getting is based on only visible type information
(if an intermediate scope had a variable "int f;" with automatic scope,
for example, the file-scope declaration wouldn't be visible, so the type
within the scope of the inner declarations should be the composite only of
the types of those declarations and not that of the file-scope
declaration).
I expect that the attribute handling currently there for built-in
functions only is because there were problems in some cases if a built-in
function were referenced without its built-in attributes. As the
attributes don't affect the function type in standard terms, it's
certainly OK, and improves diagnostic quality, to include attributes from
declarations that aren't visible.
It's possible that the piece of code you're changing always ensures that
the attributes are copied from the built-in function to the first
declaration in the inner scope (even when any file scope / external scope
declaration is shadowed), and, if composite_type and duplicate_decls
always preserve attributes, this might mean the code you're not changing
doesn't actually need its attribute handling because the attributes are
always present (for all functions, given your patch, or for built-in
functions, without it) even without that handling. So if something
changed that did make that code unnecessary, it might have been a fix in
composite_type or duplicate_decls.
The patch is OK.
--
Joseph S. Myers
jos...@codesourcery.com
