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

Reformat (and only reformat) the inline assembly chapter #1550

Merged
merged 1 commit into from
Aug 11, 2024

Conversation

traviscross
Copy link
Contributor

@traviscross traviscross commented Jul 28, 2024

The go forward plan adopted by T-spec on 2024-06-13 includes, as one step, to reformat all of the chapters of the Reference to attach named identifiers to each claim, more or less. The resulting text will use the mdbook-spec extension for rendering (see PR #1542).

Adding these named identifiers more granularly throughout the document is one step in allowing the Reference to be used as a specification for Rust in safety-critical applications.

Per our plan, we want to reformat one chapter first, to ensure our happiness with that and to perfect our process, and to then reformat (and review and merge those reformattings of) all of the rest of the Reference chapters in the same way. We discussed and imagined that this reformatting would be somewhat mechanical, and that it could be done by a technical writer with limited experience with Rust. This is what gave us confidence that this work could be hired out, completed, and reviewed on the months-scale timeline that we had set out.

As things turned out, we were fortunate enough to hire someone with significant experience with Rust. This resulted in a PR that amounts to a rewrite of the inline assembly chapter (see PR #1523). Whatever the virtues of that, it's not what we were expecting, and the degree of change in one PR has complicated and stalled effective review. This puts in jeopardy completion of the reformatting on the timeline we had expected.

Consequently, this PR takes a different approach. Here, we perform only reformatting. That is, we add identifiers to each claim, more or less, and nothing else. We change none of the verbiage in this chapter.

Contra to earlier claims that this is not possible to do, our finding, having done this, is that it works out fine. The original text was already organized reasonably well enough to just leave it in place and add the identifiers.

Is this perfect? Almost certainly not. There's always more that could be done to improve a chapter. E.g., perhaps some claims could be broken down further still and more identifiers added, though there is a distinct readability tradeoff here. We've tried to strike a reasonable balance in this PR.

The point of the exercise contained in this PR is that this diff is straightforward to review and moves us in the direction that we want to go. We can always make other changes later, and by separating them out, those changes will be easier to review also.

Note that we're unhappy with the current rendering when two identifiers need to be stacked, e.g.:

[asm]
[asm.intro]

We'll plan to improve this later and separately with work in mdbook-spec or in the style sheets.

cc @rust-lang/spec @Amanieu

r? @ehuss

@traviscross traviscross added the T-spec Team: spec label Jul 28, 2024
@traviscross
Copy link
Contributor Author

traviscross commented Jul 28, 2024

@rfcbot fcp merge

We should decide via FCP whether, as a matter of process, we want the PRs for the remaining chapters to looks like this.1 This isn't to say that we don't want other, later, PRs that may make more extensive changes. It's just to say that, as the first step, we want reformatting PRs whose diffs look like this, and that we want to see this for as many chapters of the Reference as possible.

That's what I propose that we should expect, and that's the consensus we'd be adopting here via this FCP.

Footnotes

  1. We've discussed how there might be some chapters that are a total loss and might in fact just need to be deleted. We can set aside separately these "trashcan" chapters.

@rfcbot
Copy link

rfcbot commented Jul 28, 2024

Team member @traviscross has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@ehuss
Copy link
Contributor

ehuss commented Jul 30, 2024

Checking my box since I will be away for a while.

A minor comment is that we should probably make a decision about using underscores or hyphens in the rule names. I can understand if a rule contains something that is literal in rust (like clobbers_abi) that it is natural to use underscores. My instinct is to just standardize on that, but I don't have a strong opinion. However, I do think we should have guidance to avoid inconsistent usage.

@rfcbot
Copy link

rfcbot commented Aug 1, 2024

🔔 This is now entering its final comment period, as per the review above. 🔔

psst @traviscross, I wasn't able to add the final-comment-period label, please do so.

The go forward plan adopted by T-spec on 2024-06-13 includes, as one
step, to reformat all of the chapters of the Reference to attach named
identifiers to each claim, more or less.  The resulting text will use
the `mdbook-spec` extension for rendering (see PR #1542).

Adding these named identifiers more granularly throughout the document
is one step in allowing the Reference to be used as a specification
for Rust in safety-critical applications.

Per our plan, we want to reformat one chapter first, to ensure our
happiness with that and to perfect our process, and to then
reformat (and review and merge those reformattings of) all of the rest
of the Reference chapters in the same way.  We discussed and imagined
that this reformatting would be somewhat mechanical, and that it could
be done by a technical writer with limited experience with Rust.  This
is what gave us confidence that this work could be hired out,
completed, and reviewed on the months-scale timeline that we had set
out.

As a demonstration of the first step in that plan, this PR
performs *only* reformatting.  That is, we add identifiers to each
claim, more or less, *and nothing else*.  We change *none* of the
verbiage in this chapter.

Our finding is that doing things this way works out fine.  The
original text was already organized reasonably well enough to just
leave it in place and add the identifiers.

There's always more that could be done to improve a chapter.  E.g.,
perhaps some claims could be broken down further still and more
identifiers added, though there is a distinct readability tradeoff
here.  We've tried to strike a reasonable balance in this PR.

The point of the exercise contained in this PR is that this diff is
straightforward to review and moves us in the direction that we want
to go.  We can always make other changes later, and by separating them
out, those changes will be easier to review also.

Note that we're unhappy with the current rendering when two
identifiers need to be stacked, e.g.:

```
[asm]
[asm.intro]
```

We'll plan to improve this later and separately with work in
`mdbook-spec` or in the style sheets.
@traviscross
Copy link
Contributor Author

In our 2024-08-01 spec meeting, we agreed to use kebab-case for these identifiers except when including a literal token like global_asm or preserves_flags. This PR has now been updated accordingly.

@rfcbot
Copy link

rfcbot commented Aug 11, 2024

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

psst @traviscross, I wasn't able to add the finished-final-comment-period label, please do so.

@traviscross traviscross added this pull request to the merge queue Aug 11, 2024
Merged via the queue into master with commit 62cd0df Aug 11, 2024
4 checks passed
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Aug 14, 2024
Update books

## rust-lang/book

7 commits in 67fa536768013d9d5a13f3a06790521d511ef711..04bc1396bb857f35b5dda1d773c9571e1f253304
2024-07-31 13:19:44 UTC to 2024-07-16 18:18:38 UTC

- mdbook-trpl-listing: Add missing elided lifetimes (rust-lang/book#3995)
- infra: include ghp-import and git push in generate-preview script (rust-lang/book#3998)
- infra: add robots.txt for GH Pages previews (rust-lang/book#3997)
- Clarify function definitions vs. expressions (rust-lang/book#3870)
- infra: fix some shellcheck issues in CI config (rust-lang/book#3988)
- infra: support test renderer in mdbook preprocessors (rust-lang/book#3982)
- Improve handling of `<Listing>`s (rust-lang/book#3975)

## rust-lang/edition-guide

4 commits in 5454de3d12b9ccc6375b629cf7ccda8264640aac..aeeb287d41a0332c210da122bea8e0e91844ab3e
2024-08-06 21:16:24 UTC to 2024-07-29 21:41:36 UTC

- Stabilize unsafe extern blocks (rust-lang/edition-guide#313)
- Add chapter for Lifetime Capture Rules 2024 (rust-lang/edition-guide#316)
- 2024: Add page for missing_fragment_specifier (rust-lang/edition-guide#315)
- Add documentation for 2024 prelude migration. (rust-lang/edition-guide#314)

## rust-lang/nomicon

3 commits in 0ebdacadbda8ce2cd8fbf93985e15af61a7ab895..6ecf95c5f2bfa0e6314dfe282bf775fd1405f7e9
2024-08-11 16:55:29 UTC to 2024-08-09 23:25:22 UTC

- Stabilize `min_exhaustive_patterns` (rust-lang/nomicon#445)
- repr(int) enums: both size and sign matter (rust-lang/nomicon#458)
- Update what-unsafe-does.md (rust-lang/nomicon#457)

## rust-lang/reference

6 commits in 2e191814f163ee1e77e2d6094eee4dd78a289c5b..62cd0df95061ba0ac886333f5cd7f3012f149da1
2024-08-11 21:06:12 UTC to 2024-07-30 06:34:03 UTC

- Reformat (and only reformat) the inline assembly chapter (rust-lang/reference#1550)
- Changes for unsafe extern blocks (RFC 3484) (rust-lang/reference#1536)
- Stabilize Wasm relaxed SIMD (rust-lang/reference#1421)
- Remove custom blockquote styling (rust-lang/reference#1547)
- Fix std-links for generics with commas. (rust-lang/reference#1549)
- Add details on how names are introduced. (rust-lang/reference#1052)

## rust-lang/rust-by-example

3 commits in 89aecb6951b77bc746da73df8c9f2b2ceaad494a..8f94061936e492159f4f6c09c0f917a7521893ff
2024-08-06 17:25:35 UTC to 2024-07-16 20:58:25 UTC

- Update lifetime_bounds.md (rust-lang/rust-by-example#1869)
- Remove the link to Japanese translation (rust-lang/rust-by-example#1868)
- Add an example of implementing the FromStr trait for Circles. (rust-lang/rust-by-example#1865)

## rust-lang/rustc-dev-guide

12 commits in 0c4d55cb59fe440d1a630e4e5774d043968edb3f..43d83780db545a1ed6d45773312fc578987e3968
2024-08-08 17:54:27 UTC to 2024-07-19 07:15:12 UTC

- Added 'the' in chapter "Running test" subtitle "Run unit tests on the compiler/library" (rust-lang/rustc-dev-guide#2040)
- Correct rust code block in *Dataflow Analysis* (rust-lang/rustc-dev-guide#2037)
- linkcheck: fix filtering of the source files (rust-lang/rustc-dev-guide#2019)
- chore: fix some comments (rust-lang/rustc-dev-guide#2028)
- linkcheck: fix reported broken links (part 2) (rust-lang/rustc-dev-guide#2024)
- typo (rust-lang/rustc-dev-guide#2029)
- Fix broken links in `llvm-coverage-instrumentation.md` (rust-lang/rustc-dev-guide#2027)
- Fix invalid link to toolstate documentation (rust-lang/rustc-dev-guide#2021)
- linkcheck: fix reported broken links (part 1) (rust-lang/rustc-dev-guide#2022)
- fix link (rust-lang/rustc-dev-guide#2020)
- MIR docs: fix borked links and update style (rust-lang/rustc-dev-guide#2017)
- Update adding.md (rust-lang/rustc-dev-guide#2016)
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Aug 15, 2024
Rollup merge of rust-lang#129015 - rustbot:docs-update, r=ehuss

Update books

## rust-lang/book

7 commits in 67fa536768013d9d5a13f3a06790521d511ef711..04bc1396bb857f35b5dda1d773c9571e1f253304
2024-07-31 13:19:44 UTC to 2024-07-16 18:18:38 UTC

- mdbook-trpl-listing: Add missing elided lifetimes (rust-lang/book#3995)
- infra: include ghp-import and git push in generate-preview script (rust-lang/book#3998)
- infra: add robots.txt for GH Pages previews (rust-lang/book#3997)
- Clarify function definitions vs. expressions (rust-lang/book#3870)
- infra: fix some shellcheck issues in CI config (rust-lang/book#3988)
- infra: support test renderer in mdbook preprocessors (rust-lang/book#3982)
- Improve handling of `<Listing>`s (rust-lang/book#3975)

## rust-lang/edition-guide

4 commits in 5454de3d12b9ccc6375b629cf7ccda8264640aac..aeeb287d41a0332c210da122bea8e0e91844ab3e
2024-08-06 21:16:24 UTC to 2024-07-29 21:41:36 UTC

- Stabilize unsafe extern blocks (rust-lang/edition-guide#313)
- Add chapter for Lifetime Capture Rules 2024 (rust-lang/edition-guide#316)
- 2024: Add page for missing_fragment_specifier (rust-lang/edition-guide#315)
- Add documentation for 2024 prelude migration. (rust-lang/edition-guide#314)

## rust-lang/nomicon

3 commits in 0ebdacadbda8ce2cd8fbf93985e15af61a7ab895..6ecf95c5f2bfa0e6314dfe282bf775fd1405f7e9
2024-08-11 16:55:29 UTC to 2024-08-09 23:25:22 UTC

- Stabilize `min_exhaustive_patterns` (rust-lang/nomicon#445)
- repr(int) enums: both size and sign matter (rust-lang/nomicon#458)
- Update what-unsafe-does.md (rust-lang/nomicon#457)

## rust-lang/reference

6 commits in 2e191814f163ee1e77e2d6094eee4dd78a289c5b..62cd0df95061ba0ac886333f5cd7f3012f149da1
2024-08-11 21:06:12 UTC to 2024-07-30 06:34:03 UTC

- Reformat (and only reformat) the inline assembly chapter (rust-lang/reference#1550)
- Changes for unsafe extern blocks (RFC 3484) (rust-lang/reference#1536)
- Stabilize Wasm relaxed SIMD (rust-lang/reference#1421)
- Remove custom blockquote styling (rust-lang/reference#1547)
- Fix std-links for generics with commas. (rust-lang/reference#1549)
- Add details on how names are introduced. (rust-lang/reference#1052)

## rust-lang/rust-by-example

3 commits in 89aecb6951b77bc746da73df8c9f2b2ceaad494a..8f94061936e492159f4f6c09c0f917a7521893ff
2024-08-06 17:25:35 UTC to 2024-07-16 20:58:25 UTC

- Update lifetime_bounds.md (rust-lang/rust-by-example#1869)
- Remove the link to Japanese translation (rust-lang/rust-by-example#1868)
- Add an example of implementing the FromStr trait for Circles. (rust-lang/rust-by-example#1865)

## rust-lang/rustc-dev-guide

12 commits in 0c4d55cb59fe440d1a630e4e5774d043968edb3f..43d83780db545a1ed6d45773312fc578987e3968
2024-08-08 17:54:27 UTC to 2024-07-19 07:15:12 UTC

- Added 'the' in chapter "Running test" subtitle "Run unit tests on the compiler/library" (rust-lang/rustc-dev-guide#2040)
- Correct rust code block in *Dataflow Analysis* (rust-lang/rustc-dev-guide#2037)
- linkcheck: fix filtering of the source files (rust-lang/rustc-dev-guide#2019)
- chore: fix some comments (rust-lang/rustc-dev-guide#2028)
- linkcheck: fix reported broken links (part 2) (rust-lang/rustc-dev-guide#2024)
- typo (rust-lang/rustc-dev-guide#2029)
- Fix broken links in `llvm-coverage-instrumentation.md` (rust-lang/rustc-dev-guide#2027)
- Fix invalid link to toolstate documentation (rust-lang/rustc-dev-guide#2021)
- linkcheck: fix reported broken links (part 1) (rust-lang/rustc-dev-guide#2022)
- fix link (rust-lang/rustc-dev-guide#2020)
- MIR docs: fix borked links and update style (rust-lang/rustc-dev-guide#2017)
- Update adding.md (rust-lang/rustc-dev-guide#2016)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants