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

Reorg and update attributes #537

Merged
merged 4 commits into from
Mar 17, 2019
Merged

Conversation

ehuss
Copy link
Contributor

@ehuss ehuss commented Mar 15, 2019

  • Move "Attributes" out of the "Items" chapter.
  • Move each attribute out of attributes.md, scattered to various locations throughout the reference, including a few new dedicated pages. Each attribute gets a dedicated section to make it easier to link to, and enough room to fully document.
  • See attributes.md for the consolidated built-in index.
  • Document the syntax for the inputs of every built-in attribute.
  • Minor updates and additions to various attributes.
  • Removed no_start, it is no longer used. Removed when runtime/green threads were removed Finish runtime removal rust#18967.

Things reserved for future PRs:

  • Expanding individual attributes. Some are too brief or slightly wrong.
  • Documenting the remaining undocumented attributes.
  • Finish the "Attributes on" sections to specify where each attribute may be used.

Some of the new locations seem a little awkward or arbitrary, but I couldn't think of better choices. I wanted to open this up to start getting feedback.

This is written assuming rust-lang/rust#58899 will get merged. Also, some attributes are not very strict about validating their inputs (for example, link ignores any unknown field). I documented with what I think they are intended, not what they actually accept.

Closes #534

@Centril Centril self-assigned this Mar 15, 2019
@Havvy Havvy self-assigned this Mar 15, 2019
Havvy
Havvy previously requested changes Mar 15, 2019
Copy link
Contributor

@Havvy Havvy left a comment

Choose a reason for hiding this comment

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

😍 😍 😍

@ehuss and I discussed parts of this PR on the Discord. I asked about why some new pages are under attributes.md and others are in the top-level index. @ehuss said he's like feedback on it. I'll give my specific feedback later on in the review. Next up, @ehuss asked if we should have a file system directory for each attributes page and I thought he meant like a listing of all attributes. While writing this, I figured out what he meant and I said I'd prefer one, but others may differ on opinions. After mostly just being confusing with that topic, I went to point out that "compiler attributes" is offly generic and got agreement. If anybody has a better name, let us know. I also said I think crate_name goes with recursion_limit but the actual effects of setting crate_name on the compiled code are not something I know nor does @ehuss. We finished up talking about derive, but that topic is worthy of its own issue.

With that, onto the critique parts of the review.

For the page structure (SUMMARY.md), all pages should go under attributes, or none should. If we don't make an attributes/ directory, each page name should have -attributes in the name. But I'd rather see an attributes directory. Don't do both like expressions and some items do though.

Given the only content for the code generation, testing, and diagnostics pages are attributes, it makes sense to keep them under the attributes page in the listing. The code generation page is especially confusing because we don't actually discuss what code generation happens. Are there plans to put more there? If so, we should open an issue for such a page. Same with testing and diagnostics.

I have no opinion on the attributes redirect JS.

I like making the examples for the syntaxes of attributes into a table. I was initially disliking linking the to the syntactic name for the syntax of each individual attribute, but with the examples of each attribute also being there, I'm okay with it being a bit technical.

I don't want the TODOs to be merged in. File issues for those.

src/attributes.md Outdated Show resolved Hide resolved
src/attributes.md Outdated Show resolved Hide resolved
src/codegen.md Outdated Show resolved Hide resolved
src/codegen.md Outdated Show resolved Hide resolved
src/compiler-attrs.md Outdated Show resolved Hide resolved
src/items/external-blocks.md Show resolved Hide resolved
src/runtime.md Outdated Show resolved Hide resolved
src/runtime.md Outdated Show resolved Hide resolved
src/testing.md Outdated Show resolved Hide resolved
src/unsafety.md Outdated Show resolved Hide resolved
src/abi.md Show resolved Hide resolved
src/abi.md Show resolved Hide resolved
- [`allow`], [`warn`], [`deny`], [`forbid`] — Alters the default lint level.
- [`deprecated`] — Generates deprecation notices.
- [`must_use`] — Generates a lint for unused values.
- ABI, Linking, Symbols, and FFI
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have any method to the ordering of the bullets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:) I was wondering if anyone would notice that. The sections are ordered based on how many crates on crates.io use the attributes. It's somewhat arbitrary, and not really intended as a formal ordering (mostly just "important things towards the top"). I liked the idea of grouping them, but alphabetizing the groups didn't seem satisfying. I'm up for any ordering, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

=P

The sections are ordered based on how many crates on crates.io use the attributes.

This was not a possibility I had considered 😂

I liked the idea of grouping them, but alphabetizing the groups didn't seem satisfying.

Yeah I alphabetized in my head but it felt unsatisfying to me also.

I'm up for any ordering, though.

Cool, let's perhaps file as an issue to consider later?

src/attributes.md Outdated Show resolved Hide resolved
src/codegen.md Outdated Show resolved Hide resolved
src/codegen.md Outdated Show resolved Hide resolved
src/codegen.md Outdated Show resolved Hide resolved
src/codegen.md Outdated Show resolved Hide resolved
src/codegen.md Outdated Show resolved Hide resolved
src/compiler-attrs.md Outdated Show resolved Hide resolved
- Various fixes.
- Move new pages to an `attributes` subdirectory.
- Add some introductory text to new pages.
- Remove "closure" from `cold` and `inline`, it is not stable.
- Remove TODOs.
- Added link for `feature`.
src/conditional-compilation.md Show resolved Hide resolved
src/derive.md Outdated Show resolved Hide resolved
src/derive.md Outdated Show resolved Hide resolved
src/derive.md Outdated Show resolved Hide resolved
src/diagnostics.md Outdated Show resolved Hide resolved
src/runtime.md Outdated Show resolved Hide resolved
src/testing.md Outdated Show resolved Hide resolved
src/testing.md Outdated Show resolved Hide resolved
src/testing.md Outdated Show resolved Hide resolved
src/testing.md Outdated Show resolved Hide resolved
@ehuss
Copy link
Contributor Author

ehuss commented Mar 16, 2019

I think I got most of Centril's points. I renamed the section with no name to "Limits". Still can't think of anything better.

The main points still not addressed:

I'll file some issues for some of the todo items soon. (done)

src/attributes/limits.md Show resolved Hide resolved
- Runtime
- [`panic_handler`] — Sets the function to handle panics.
- [`global_allocator`] — Sets the global memory allocator.
- [`windows_subsystem`] — Specifies the windows subsystem to link with.
- Features
- `feature` — Used to enable unstable or experimental compiler features. See
[The Unstable Book] for features implemented in `rustc`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think perhaps B-unstable is more accurate. Consider linking to it as well?

}
```

When used on a function in an implementation, the attribute does nothing.
When used on a function in a trait implementation, the attribute does nothing.
Copy link
Contributor

@Centril Centril Mar 16, 2019

Choose a reason for hiding this comment

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

(The comment below is not relevant for this PR)

@varkor @oli-obk We could reasonably make this do something by having the #[must_use] trigger when the concrete type is known? E.g.

trait A { fn b() -> usize; }

struct C;

impl A for C {
    #[must_use]
    fn b() -> usize { 0 }
}

fn main() {
    C::b();
//  ^--- We know statically that `C::b` references the function
//       on `impl A for C { ... }` and so we may trigger the lint...?
}

@Centril Centril merged commit c207931 into rust-lang:master Mar 17, 2019
You can implement `derive` for your own traits through [procedural macros].
The [`cfg`] and [`cfg_attr`] attributes are active. The [`test`] attribute is
inert when compiling for tests and active otherwise. [Attribute macros] are
active. All other attributes are inert.
Copy link
Contributor

Choose a reason for hiding this comment

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

test is always active, when not compiling for tests it transforms its input into nothing and disappears itself too.
Also, derive is an active attribute too.

Centril added a commit to Centril/rust that referenced this pull request Mar 29, 2019
Update books

Update reference, book, rust-by-example, edition-guide, embedded-book

## reference

15 commits in 41493ff..27ad493
2019-03-05 12:32:22 +0100 to 2019-03-26 02:06:15 +0100
- Document wasm_import_module for #[link]. (rust-lang/reference#554)
- Fix tidy error. (rust-lang/reference#552)
- Some minor contributing updates. (rust-lang/reference#551)
- Document `type_length_limit`. (rust-lang/reference#546)
- Add some terms to the glossary. (rust-lang/reference#547)
- Document `target_feature` and `cfg_target_feature`. (rust-lang/reference#545)
- Remove undocumented page (rust-lang/reference#539)
- Reorg and update attributes (rust-lang/reference#537)
- Fix some minor link errors. (rust-lang/reference#538)
- Add linkchecker. (rust-lang/reference#521)
- Expand docs on Macros By Example. (rust-lang/reference#511)
- document #[panic_handler] (rust-lang/reference#362)
- document #[used] (rust-lang/reference#361)
- Note that UB is program-global (rust-lang/reference#490)
- Fix copy-paste error in procedural-macros.md (rust-lang/reference#533)

## book

16 commits in 9cffbeabec3bcec42d09432bfe7705125c848889..b93ec30bbc7b1b5c2f44223249ab359bed2ed5a6
2019-03-02 08:22:41 -0500 to 2019-03-26 16:54:10 -0400
- Unignore example that now compiles
- Fix code snippet (rust-lang/book#1863)
- Fix mdbook link text in readme (rust-lang/book#1881)
- Wrap to 80 cols
- Make sentence more complete (rust-lang/book#1885)
- consistenly use increment and decrement (rust-lang/book#1884)
- Fix link to Reference's conditional-compilation. (rust-lang/book#1878)
- Fix subject/verb agreement
- Remove nostarch snapshot files that have been incorporated and checked
- haha teach the dictionary steve's name
- Add authorship info to the front page
- fix accidental <ol>'s (rust-lang/book#1866)
- Edits to Macros (rust-lang/book#1848)
- Mention `lock` returns `MutexGuard` wrapped in a `LockResult`
- Add an example that illustrates NLL (rust-lang/book#1842)
- change the parameter name from `type` to `kind` (rust-lang/book#1845)

## rust-by-example

33 commits in 2ce92beabb912d417a7314d6da83ac9b50dc2afb..f68ef3d0f4959f6a7d92a08d9994b117f0f4d32d
2018-11-20 10:10:23 -0500 to 2019-03-12 15:32:12 -0300
- Fix some broken links. (rust-lang/rust-by-example#1161)
- Update links in README (rust-lang/rust-by-example#1167)
- Add score/lifetimes/trait.md (rust-lang/rust-by-example#1168)
- Fix rust-lang/rust-by-example#1147 - No more `open_mode` method (rust-lang/rust-by-example#1164)
- Fix for loop description in list print example (rust-lang/rust-by-example#1162)
- Add link to Cargo chapter in the index page (rust-lang/rust-by-example#1159)
- Fix grammar in sentence about integer notation (rust-lang/rust-by-example#1157)
- Do not use deprecated functions from `std::error::Error` trait (rust-lang/rust-by-example#1151)
- Update new_types.md to clarify conversion to base type (rust-lang/rust-by-example#1148)
- Fix compatibility with Rust 2018 (rust-lang/rust-by-example#1150)
- Hello: Fix hint link in `fmt` chapter. (rust-lang/rust-by-example#1146)
- Clarify pub(restricted) example a bit (rust-lang/rust-by-example#1133)
- Add "literal" to list of macro designators (rust-lang/rust-by-example#1153)
- Minor fixes for the macros chapter (rust-lang/rust-by-example#1113)
- Use new book links instead of the old second-edition ones (rust-lang/rust-by-example#1143)
- Recommend implementing Display over ToString (rust-lang/rust-by-example#1145)
- Remove unused import and format with `rustfmt` (rust-lang/rust-by-example#1144)
- fix typo (rust-lang/rust-by-example#1142)
- Update syntax for 2018 Edition (rust-lang/rust-by-example#1136)
- Added two missing full stops (rust-lang/rust-by-example#1138)
- Removed unnecessary spaces before macro designators in macros/dry (rust-lang/rust-by-example#1139)
- fix install mdbook command (rust-lang/rust-by-example#1128)
- Changed word `function` to `type` in comment of fn area (rust-lang/rust-by-example#1132)
- Added two missing backticks in generics/multi_bounds (rust-lang/rust-by-example#1129)
- Fixed small logic error in error/option_unwrap/and_then (rust-lang/rust-by-example#1127)
- Fix typo (rust-lang/rust-by-example#1125)
- The code of conduct link was dead. I fixed it. (rust-lang/rust-by-example#1122)
- I added a space in the Display fmt for Complex (rust-lang/rust-by-example#1123)
- Fix Rust install link in the index (rust-lang/rust-by-example#1124)
- Update cargo conventions section (rust-lang/rust-by-example#1121)
- Fixed curly braces in the `To and from Strings` chapter to be parentheses (rust-lang/rust-by-example#1120)
- Edit a typo (rust-lang/rust-by-example#1119)
- Fixes rust-lang/rust-by-example#1115 by correcting the typo from into_iterator to into_iter (rust-lang/rust-by-example#1118)

## edition-guide

1 commits in aa0022c875907886cae8f3ef8e9ebf6e2a5e728d..b56ddb11548450a6df4edd1ed571b2bc304eb9e6
2019-02-27 22:10:39 -0800 to 2019-03-10 10:23:16 +0100
- Links fixes (rust-lang/edition-guide#133)

## embedded-book

6 commits in 9e656ead82bfe869493dec82653a52e27fa6a05c..07fd3880ea0874d82b1d9ed30ad3427ec98b4e8a
2019-03-03 16:03:26 +0000 to 2019-03-27 15:40:52 +0000
- Fix test errors.  (rust-embedded/book#180)
- Update qemu.md  (rust-embedded/book#170)
- Update no-std.md to remove obsolete FAQ link  (rust-embedded/book#177)
- We've come a long way :)  (rust-embedded/book#176)
- Correct link to team  (rust-embedded/book#175)
- Update some book links to their new homes.  (rust-embedded/book#173)
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.

4 participants