r335084 - Append new attributes to the end of an AttributeList.

[Michael Kruse via cfe-commits]

Hi,

multiple comments in the code indicate that the attribute order was
surprising and probably has lead to bugs, and will lead to bugs in the
future. The order had to be explicitly reversed to avoid those. This
alone for me this seems a good reason to have the attributes in the
order in which they appear in the source.

It would be nice it you could send a reproducer. At a glance, your
case does not seem related since the format strings are function
arguments, not attributes.

Michael


2018-06-23 17:17 GMT-05:00 David Jones <dlj at google.com>:
> This commit seems to have some substantial downsides... for example, it now
> flags calls like this as warnings:
> http://git.savannah.gnu.org/cgit/gettext.git/tree/gettext-tools/src/msgl-check.c?id=05ecefa943f339019b7127aa92cbb09f6fd49ed3#n478
>
> Previously, the reverse order meant that the plural format string was
> examined, but now it is only the singular string. Since the singular string
> doesn't include a substitution, the unused format variable warning fires
> after this revision.
>
> I don't see a strong argument for why one order is more correct than the
> other; however, given that this is in conflict with real code found in the
> wild, I think this needs to be addressed.
>
> Since the semantics of the ordering of multiple format args seems somewhat
> ill-defined, it seems to me like reverting may be the best near-term choice.
> I can imagine other ways to fix the diagnostic, but the only behaviour that
> I would believe to be expected by existing code is the old one, so a change
> like this one probably needs to be more carefully vetted before being
> (re-)committed.

Could you give more details about your concerns?


> Please let me know your plan. (If I don't hear back in a day or so, I'll go
> ahead and revert for you as the safe default course of action.)

On a weekend?