[PATCH] Add default parameter to MCSymbol accessors to control IsUsed

Rafael EspĂ­ndola via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 7 12:03:51 PDT 2015


+    IsUsed = SetUsed;

Should this be

IsUsed |= SetUsed;

instead? Otherwise you will also clear the IsUsed bit, which is
probably not the intention.

Can you leave the old tests to show that now we don't error on them?






On 6 August 2015 at 18:14, Vedant Kumar <vsk at apple.com> wrote:
> Hi,
>
> Currently, some accessors in MCSymbol set IsUsed when they shouldn't be. This can lead to surprising bugs.
>
> The first patch I've attached adds a default parameter, "SetUsed = true", to some MCSymbol accessors. This preserves the current behavior (NFC) without impacting existing code. It also buys us more flexibility when we don't want to set IsUsed.
>
> The second patch touches MC/AsmParser to fix an issue where we get "invalid reassignment of non-absolute variable" when the error doesn't apply.
>
> The second patch deserves closer scrutiny, because, try as I might, I couldn't craft a test case which gets this error to trigger. I need some help coming up for a test case for it. It could be the case that the error should never trigger -- if so, we can get rid of it.
>
> ninja check-all reports no issues.
>
> I don't have commit rights. I'd appreciate someone reviewing all of this and (hopefully) guiding it into trunk.
>
> thanks!
> vedant
>
>
>
>
>


More information about the llvm-commits mailing list