On Wed, 16 Jan 2019, Martin Sebor wrote:
> + /* Iterate over tentative records (all those at the head of the list
> + with a null FUNCTION) and either associate them with DECL when ADD
> + is set or remove them from it otherwise. */
> + for (c_inline_static *csi = c_inline_statics, *last = csi;
> + csi; csi = csi->next)
(Here we start with last == c_inline_statics.)
> + if (add)
> + {
> + if (csi->static_decl)
> + csi->function = decl;
> + }
I don't see any way in which csi->static_decl can be NULL (both callers of
record_inline_static appear only to use non-NULL values for that argument,
and it being non-NULL seems implicit in the semantics of the function
given by the comment above it). That is, I don't see the use of the inner
"if" here.
> + if (last == c_inline_statics)
> + c_inline_statics = last = csi->next;
> + else
> + last->next = csi->next;
Here, if either last or c_inline_statics changes, they remain the same, so
I don't see how they can ever be different.
So I don't see the need for having the variable last at all, or the
conditional "if (last == c_inline_statics)". It's always the case that,
if removing entries from the list, it's some number of leading entries, so
I'd expect just "c_inline_statics = csi->next;" to be sufficient for
removing an entry.
> + /* FUNCTION is unset for a declaration whose signature references
> + a static variable and for which a definition wasn't provided. */
> + if (!csi->function)
> + continue;
But those should have been removed after the declaration in all cases via
a call to update_tentative_inline_static. So why is this conditional
needed here?
> + /* Only extern functions are of interest. STATIC_DECL is null
> + for inline functions that have been defined that contain
> + no references to statics (but whose subsequent declarations
> + might). */
> + if (!DECL_EXTERNAL (csi->function)
> + || !csi->static_decl)
> + continue;
As above, I don't see the need for the !csi->static_decl check, as
csi->static_decl should never be NULL.
--
Joseph S. Myers
[email protected]
