[PATCH] D90108: [MC] Error for .weak/.globl/.local which change the symbol binding

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 28 10:41:22 PDT 2020


MaskRay added a comment.

In D90108#2358482 <https://reviews.llvm.org/D90108#2358482>, @jhenderson wrote:

> In D90108#2357342 <https://reviews.llvm.org/D90108#2357342>, @nickdesaulniers wrote:
>
>> In D90108#2357336 <https://reviews.llvm.org/D90108#2357336>, @MaskRay wrote:
>>
>>> We can surely loose it if it turns out to be a wide spreading problem.
>>
>> The hard part is that regressions tend to get reported AFTER the release when it's too late to fix them.  Maybe a warning for one release, then upgrade to hard error next release gives users more time to find and fix errors, and doesn't result in an unusable release version for anyone?
>
> I don't have any specific evidence to suggest that we'd run into a problem doing this, but my personal thinking is that adding an option to allow disabling this error might be a good idea. I wouldn't be surprised to find code bases where fixing the code is non-trivial. Ideally, the option would not be made widely known unless actually asked for and/or heavily labelled with things saying something like "this is a temporary option that will be deleted in a future change unless a compelling reason is found not to".

If there are concerns, I can make `.globl foo; .weak foo` a warning instead of error. The others are still errors.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90108/new/

https://reviews.llvm.org/D90108



More information about the llvm-commits mailing list