[PATCH] D101068: [ELF] Emit diagnostic for invalid size directive
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Apr 27 09:09:51 PDT 2021
MaskRay added a comment.
In D101068#2719743 <https://reviews.llvm.org/D101068#2719743>, @bcain wrote:
> In D101068#2718612 <https://reviews.llvm.org/D101068#2718612>, @MaskRay wrote:
>
>> Yeah, I kinda feel like we are not making good use of the 50 lines of code. .size is too minor a feature that I'd waste 50 lines, adding `MCAssembler &Asm,` in various places to make this work.
>> To make good use of the 50 lines, this should be something generic (think as a utility similar to `evaluateAsRelocatable`), so that every other directive which has similar usage to .size can benefit from this.
>
> Hmm. Removing the spell check portion of NoteUndef only saves around ~9 lines. So it sounds like that's not sufficient? You prefer either not accepting any of this patch or expanding its scope to include other directives? I will explore the latter.
Yes, reusing this in more places can improve its value... I am also a bit unsure about `did you mean to refer` suggesting more than one symbol.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101068/new/
https://reviews.llvm.org/D101068
More information about the llvm-commits
mailing list