Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass self.definitions to armasm[64].exe. #868

Merged
merged 1 commit into from
Sep 23, 2023

Conversation

dot-asm
Copy link
Contributor

@dot-asm dot-asm commented Sep 21, 2023

No description provided.

@thomcc
Copy link
Member

thomcc commented Sep 21, 2023

Do you have a link to docs for this?

@dot-asm
Copy link
Contributor Author

dot-asm commented Sep 21, 2023

@dot-asm
Copy link
Contributor Author

dot-asm commented Sep 21, 2023

Well, the ARM doc talks about --predefine but for some reason so-far-case-insensitive MS decided that they accept specifically --PreDefine. This is in accordance with armasm[64] -h.

@dot-asm
Copy link
Contributor Author

dot-asm commented Sep 21, 2023

Just in case, this is less important than #867.

@dot-asm
Copy link
Contributor Author

dot-asm commented Sep 22, 2023

It occurred to me that you might have meant to suggest to add a commentary with the reference to the docs. Is this it?

@thomcc
Copy link
Member

thomcc commented Sep 23, 2023

No, I wanted the docs:

https://developer.arm.com/documentation/dui0802/b/Directives-Reference/SETA--SETL--and-SETS?lang=en
and
https://developer.arm.com/documentation/dui0802/b/armasm-Command-line-Options/--predefine

were what I was looking for. Thanks. I don't think a link in the code is needed -- my experience with ARM's docs are that the links don't tend to be that stable, so it'd probably bitrot.

Copy link
Member

@thomcc thomcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thomcc thomcc merged commit 7ee0f86 into rust-lang:main Sep 23, 2023
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants