[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