[PATCH] D92507: [ms] [llvm-ml] Add support for ALIGN, EVEN, and ORG directives

[Eric Astor via Phabricator via llvm-commits]

epastor added a comment.

In D92507#2448819 <https://reviews.llvm.org/D92507#2448819>, @thakis wrote:

> In D92507#2445774 <https://reviews.llvm.org/D92507#2445774>, @epastor wrote:
>
>> In D92507#2431130 <https://reviews.llvm.org/D92507#2431130>, @thakis wrote:
>>
>>> Does it wrap around 32-bit in 64-bit output too?
>>
>> Oddly enough: yes.
>>
>>> Is this relied on? Or could we error out on negative values instead?
>>
>> I don't think we can prove that it isn't relied on, especially when ORG explicitly allows putting things "outside bounds", so I'd rather maintain compatibility.
>
> Well, we don't need to prove it if we make it an error: We'll know if the diag fires. So if we make this an error first and then build lots of files with it, we'll learn if we run into it. At that point we'll know we need to make it compatible, but maybe we won't have to. Doesn't it seem better to default to less surprising behavior and only put in the compat warts once there's evidence that we need them?

I'm not convinced on the philosophical grounds... but I went poking around GitHub for evidence, and couldn't find any uses of ORG inside a STRUCT with a negative offset. So - made into an error!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92507