LGTM, but I'll defer to hans here since he's guided thus far.

In D126425#3545711, @hans wrote:

It appears to me that MASM doesn't care much about the file extension at all:

[test results omitted]

The fact that /Ta exists suggest maybe it did care at some point, but that may have changed.

Looks good to me, but probably make sure Hans agrees first.

I'm fine with this and want to remove my request for changes - but please make sure Hans approves as well.

Thanks!

In D109425#2989346, @peter.smith wrote:

In D109425#2989305, @epastor wrote:

This code is similar to code in AsmParser.cpp (https://github.com/llvm-mirror/llvm/blob/master/lib/MC/MCParser/AsmParser.cpp#L3344-L3351) - looks like that will probably need an update as well, to keep MaxBytesToFill from being interpreted as a pointer?

Yes, I managed to catch that one in D45961 that reference from llvm-mirror might be a little bit out of date. I think https://github.com/llvm/llvm-project/blob/main/llvm/lib/MC/MCParser/AsmParser.cpp#L3457 is where the code is now. Hope I've not missed something.

This code is similar to code in AsmParser.cpp (https://github.com/llvm-mirror/llvm/blob/master/lib/MC/MCParser/AsmParser.cpp#L3344-L3351) - looks like that will probably need an update as well, to keep MaxBytesToFill from being interpreted as a pointer?

Fix copy/paste error in test label

Rebase on parent

Add all-lowercase test for a mixed-case macro

Switch back to gmtime/localtime (not thread-safe, but in a generally-safe location - and more cross-platform)

Rebase on parent

Rebase on parent

Addressed feedback (and rebased on HEAD)

Add -timestamp and -utc flags to support deterministic builds

Restrict further to only named-variable references

Rebase on parent

Fix symbol usage bug

Rebase on parent

Fix accidental fold with the child commit

Fix an accidental import of MasmParser.cpp from a child commit

Rebase on parent

Rebase on parent

Address feedback (including clang-tidy)

In D104964#2853790, @thakis wrote:

Can you check if ml.exe allows defining a local variable overriding e.g. @Line? What happens then? Does it error? Can you add a test for that?

Error on redefinition of built-in symbols (matching ML.EXE)

Rebase on trunk to re-trigger build (after removing parent commit)

Added comment explaining anonymous substructures

Addressed comments (removed copy/paste cruft that proved unnecessary)

In D104194#2815762, @MaskRay wrote:

More concretely, branch instruction tests in llvm/test/MC/X86/x86-32.s can be moved outside, then you can add a test for jecxz. jexcz seems untested.

Add test case

Dropping unnecessary implicit-check-not guard

Directly check for the warning, rather than converting it to an error

This required a few too many changes; I'll re-upload a working variant on top of a prerequisite (D103993).

Fix handling of the second (and later) field after an ORG directive

While I was working on the comments above, I discovered a problem - after improving the tests and fixing various errors, the way we were dealing with determining field offsets caused a failure on the second field after an ORG offset change. I've been working on and off to adjust the code to handle this; it requires a nontrivial rework, as Structure has to track both its Size and the offset of the next field. I'll upload that revision shortly, and I'm afraid it will require a re-review. Sorry, thakis - and I'll try to find someone else if more appropriate.

In D92507#2448819, @thakis wrote:

In D92507#2445774, @epastor wrote:

In 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?

Fix lint