[PATCH] D44214: Improve --warn-symbol-ordering.
Rafael Ávila de Espíndola via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Mar 8 09:29:17 PST 2018
rafael added a comment.
James Henderson via Phabricator <reviews at reviews.llvm.org> writes:
> jhenderson added a reviewer: espindola.
> jhenderson accepted this revision.
> jhenderson added a subscriber: espindola.
> jhenderson added a comment.
> This revision is now accepted and ready to land.
>
> LGTM (with nit). Having thought about it, I agree with this comment by @espindola:
>
>> I am OK with warning on both cases, but it feels like that would be a
>>
>> separate warning. A non existing symbol showing up in the order file
>> probably means there is something wrong with how that files was
>> created. Two symbols that can't be independently moved because they are
>> in the same section (because of ICF or not) seems like a normal event in
>> most cases.
>
> I also think it would be fairly simple to add - just detect if the priority was the default or not prior to changing it (though we'd need more info to get a better warning message. I don't mind this being spun off into a separate discussion.
>
> P.S. Rafael - Can you confirm which of the two usernames you'd like us to use, please?
The "espindola" account. I am terribly sorry for the noise. I wanted to
convert the older account to not use a google sign in but could not find
out how to do it.
Cheers,
Rafael
https://reviews.llvm.org/D44214
More information about the llvm-commits
mailing list