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.

Sign up for GitHub

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

Jump to bottom

Change the syntax of the internal weak! macro #138988

Merged
merged 1 commit into from
Mar 29, 2025
Merged

Change the syntax of the internal weak! macro #138988

merged 1 commit into from
Mar 29, 2025
+144 −80

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Mar 26, 2025

Change the syntax to include parameter names and a trailing semicolon.

Motivation:

  • Mirror the syscall! macro.
  • Allow rustfmt to format it (when wrapped in parentheses, and when not inside cfg_if!).
  • For better documentation (having the parameter names available in the source code is a bit nicer).
  • Allow a future improvement to this macro where we can sometimes use the symbol directly when it's statically known to be available (and thus need the parameter names to be available), see Use cfg_os_version_min in std #136868.

r? libs

@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 26, 2025
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@madsmtm
Change the syntax of the internal weak! macro
a7bafc0 
Change the syntax to include parameter names and a trailing semicolon.

Motivation:
- Mirror the `syscall!` macro.
- Allow rustfmt to format it (when wrapped in parentheses).
- For better documentation (having the parameter names available in
  the source code is a bit nicer).
- Allow future improvements to this macro where we can sometimes use the
  symbol directly when it's statically known to be available.
@madsmtm madsmtm force-pushed the internal-weak-macro-syntax branch from 1592f5c to a7bafc0 Compare March 26, 2025 15:26
@thaliaarchi
Copy link
Contributor

I like this idea. I actually noticed recently too, but I took a slightly different approach in a draft patch. Since (-style macros look like calls, but these are declarations akin to an extern block, I changed the weak! usages to {-style to match syscall!. Is there a reason you picked (-style?

Also, I like that you're adding parameter names.

@madsmtm
Copy link
Contributor Author

madsmtm commented Mar 26, 2025

Is there a reason you picked (-style?

Yes: rustfmt automatically formats the contents of those.

@ibraheemdev
Copy link
Member

This makes sense to me, thanks! @bors r+ rollup

@bors
Copy link
Contributor

bors commented Mar 28, 2025

📌 Commit a7bafc0 has been approved by ibraheemdev

It is now in the queue for this repository.

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 28, 2025
@thaliaarchi thaliaarchi mentioned this pull request Mar 29, 2025
Open
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 29, 2025
@jhpratt
Rollup merge of rust-lang#138988 - madsmtm:internal-weak-macro-syntax…
9da804c 
…, r=ibraheemdev

Change the syntax of the internal `weak!` macro

Change the syntax to include parameter names and a trailing semicolon.

Motivation:
- Mirror the `syscall!` macro.
- Allow rustfmt to format it (when wrapped in parentheses, and when not inside `cfg_if!`).
- For better documentation (having the parameter names available in the source code is a bit nicer).
- Allow a future improvement to this macro where we can sometimes use the symbol directly when it's statically known to be available (and thus need the parameter names to be available), see rust-lang#136868.

r? libs
@jhpratt jhpratt mentioned this pull request Mar 29, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 29, 2025
@bors
Auto merge of rust-lang#139094 - jhpratt:rollup-usgpag3, r=jhpratt
31ded84 
Rollup of 8 pull requests

Successful merges:

 - rust-lang#138692 (Reject `{true,false}` as revision names)
 - rust-lang#138757 (wasm: increase default thread stack size to 1 MB)
 - rust-lang#138832 (Start using `with_native_path` in `std::sys::fs`)
 - rust-lang#138988 (Change the syntax of the internal `weak!` macro)
 - rust-lang#139044 (bootstrap: Avoid cloning `change-id` list)
 - rust-lang#139056 (use `try_fold` instead of `fold`)
 - rust-lang#139057 (use `slice::contains` where applicable)
 - rust-lang#139086 (Various cleanup in ExprUseVisitor)

r? `@ghost`
`@rustbot` modify labels: rollup
jhpratt added a commit to jhpratt/rust that referenced this pull request Mar 29, 2025
@jhpratt
Rollup merge of rust-lang#138988 - madsmtm:internal-weak-macro-syntax…
5c4b1a3 
…, r=ibraheemdev

Change the syntax of the internal `weak!` macro

Change the syntax to include parameter names and a trailing semicolon.

Motivation:
- Mirror the `syscall!` macro.
- Allow rustfmt to format it (when wrapped in parentheses, and when not inside `cfg_if!`).
- For better documentation (having the parameter names available in the source code is a bit nicer).
- Allow a future improvement to this macro where we can sometimes use the symbol directly when it's statically known to be available (and thus need the parameter names to be available), see rust-lang#136868.

r? libs
@jhpratt jhpratt mentioned this pull request Mar 29, 2025
Closed
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 29, 2025
@bors
Auto merge of rust-lang#139096 - jhpratt:rollup-nfyct4q, r=jhpratt
ef8b5e1 
Rollup of 7 pull requests

Successful merges:

 - rust-lang#138692 (Reject `{true,false}` as revision names)
 - rust-lang#138757 (wasm: increase default thread stack size to 1 MB)
 - rust-lang#138988 (Change the syntax of the internal `weak!` macro)
 - rust-lang#139044 (bootstrap: Avoid cloning `change-id` list)
 - rust-lang#139056 (use `try_fold` instead of `fold`)
 - rust-lang#139057 (use `slice::contains` where applicable)
 - rust-lang#139086 (Various cleanup in ExprUseVisitor)

r? `@ghost`
`@rustbot` modify labels: rollup
@matthiaskrgr matthiaskrgr mentioned this pull request Mar 29, 2025
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 29, 2025
@bors
Auto merge of rust-lang#139101 - matthiaskrgr:rollup-zhu7hf6, r=matth…
5cc6072 
…iaskrgr

Rollup of 7 pull requests

Successful merges:

 - rust-lang#138692 (Reject `{true,false}` as revision names)
 - rust-lang#138757 (wasm: increase default thread stack size to 1 MB)
 - rust-lang#138988 (Change the syntax of the internal `weak!` macro)
 - rust-lang#139056 (use `try_fold` instead of `fold`)
 - rust-lang#139057 (use `slice::contains` where applicable)
 - rust-lang#139086 (Various cleanup in ExprUseVisitor)
 - rust-lang#139097 (Add more tests for pin!().)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 111351f into rust-lang:master Mar 29, 2025
6 checks passed
@rustbot rustbot added this to the 1.88.0 milestone Mar 29, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 29, 2025
@rust-timer
Unrolled build for rust-lang#138988
709ec22 
Rollup merge of rust-lang#138988 - madsmtm:internal-weak-macro-syntax, r=ibraheemdev

Change the syntax of the internal `weak!` macro

Change the syntax to include parameter names and a trailing semicolon.

Motivation:
- Mirror the `syscall!` macro.
- Allow rustfmt to format it (when wrapped in parentheses, and when not inside `cfg_if!`).
- For better documentation (having the parameter names available in the source code is a bit nicer).
- Allow a future improvement to this macro where we can sometimes use the symbol directly when it's statically known to be available (and thus need the parameter names to be available), see rust-lang#136868.

r? libs
@madsmtm madsmtm deleted the internal-weak-macro-syntax branch March 29, 2025 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@ibraheemdev ibraheemdev

Labels
O-unix Operating system: Unix-like S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.88.0
Development

Successfully merging this pull request may close these issues.
6 participants
@madsmtm @rust-log-analyzer @thaliaarchi @ibraheemdev @bors @rustbot