[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