[WIP] Massive cosmetic PR, take 2 #58036

alexreg · 2019-01-31T22:58:48Z

This is my second take on #57016, where I am doing a few things differently:

Splitting into many commits, depending on area of codebase and type of change. (I shall be doing this over the coming days.)
Excluding more controversial changes from previous attempt.
Factoring off subsets of commits into separate PRs to get met reviewed and merged separately, as needed.

This is sort of like a "tracking PR" for now, and probably shouldn't be merged as-is unless someone is happy with all my changes.

CC @nikomatsakis @steveklabnik

rust-highfive · 2019-01-31T22:58:59Z

r? @pnkfelix

(rust_highfive has picked a reviewer for you, use r? to override)

pnkfelix · 2019-02-01T11:24:25Z

I'm going to put WIP in the title to reflect that this is "a 'tracking PR' for now" that shoud not be merged as is.

bors · 2019-02-01T15:35:14Z

☔ The latest upstream changes (presumably #57916) made this pull request unmergeable. Please resolve the merge conflicts.

alexreg · 2019-02-01T16:23:42Z

@pnkfelix Sure. I’ll try to finish the commit splitting later today. In the meanwhile, please let me if I can reorganise things any mire to help get this merged.

oli-obk

I'm not quite clear what the rules are where comments start with a capitalized letter and where they start with a lower case letter. As far as I can see there's both happening.

I reviewed the first commit, but I might have missed some things

src/librustc/traits/auto_trait.rs

src/librustc/traits/mod.rs

src/librustc/traits/object_safety.rs

src/librustc/traits/select.rs

oli-obk · 2019-02-01T15:53:34Z

src/librustc/ty/context.rs

@@ -835,7 +835,7 @@ pub type CanonicalUserType<'gcx> = Canonical<'gcx, UserType<'gcx>>;

 impl CanonicalUserType<'gcx> {
    /// Returns `true` if this represents a substitution of the form `[?0, ?1, ?2]`,
-    /// i.e. each thing is mapped to a canonical variable with the same index.
+    /// i.e., each thing is mapped to a canonical variable with the same index.


these ., look really weird to me.

It's weird, but it's standard in U.S. English. :-) Not in British English. The American motivation is that you're abbreviating "id est, ...", which is Latin for "that is, ...". In that sense, it makes sense, but I know what you mean still.

oli-obk · 2019-02-01T16:03:03Z

src/librustc_apfloat/ieee.rs

@@ -186,7 +186,7 @@ impl Semantics for X87DoubleExtendedS {
    ///  exponent = all 1's, integer bit 0, significand 0 ("pseudoinfinity")


I think we're supposed to keep the librustc_apfloat crate (docs) in sync with llvm's. So the best course of action would be an additional patch against llvm.

Hard to tell... it looks like some docs have already been Rustified.

src/libsyntax/parse/parser.rs

src/build_helper/lib.rs

src/liballoc/collections/btree/set.rs

src/liballoc/tests/lib.rs

src/bootstrap/builder.rs

src/liballoc/collections/btree/node.rs

bors · 2019-02-03T03:22:03Z

☔ The latest upstream changes (presumably #58079) made this pull request unmergeable. Please resolve the merge conflicts.

steveklabnik · 2019-02-04T10:28:54Z

src/tools/tidy/src/features.rs

@@ -327,7 +327,7 @@ fn map_lib_features(base_src_path: &Path,
            }
            becoming_feature = None;
            if line.contains("rustc_const_unstable(") {
-                // `const fn` features are handled specially.
+                // Const fn features are handled specially.


I'm not sure this is a good change.

Okay, happy to undo!

On second thought, I ran a search, and it seems "const fn" without the quotes is much more common in comments. I think it's okay personally, since "fn" can be considered an abbreviation for "function", and "const" and attribute/adjective.

(But I'll still revert it if you prefer.)

I'm not entirely sure that commonality is the right argument here. I don't think that either of these is particularly bad, but given that it's a specific rust feature, and that's it's name, const fn seems like its "proper name". Dunno.

It might not be... I am just of the mind to opt for consistency when in doubt as to which is "morally" better. "Fn" is used as an abbreviation all over the codebase without backticks as well, it's worth saying. Will leave this for now, but just tell me to revert if you decide on that. Just pushed BTW, so feel free to continue reviewing. Thanks!

src/tools/tidy/src/style.rs

alexreg · 2019-02-04T10:31:57Z

(About to push some changes, FYI. @steveklabnik)

bors · 2019-02-05T08:25:02Z

☔ The latest upstream changes (presumably #57973) made this pull request unmergeable. Please resolve the merge conflicts.

bors · 2019-02-17T10:18:25Z

☔ The latest upstream changes (presumably #58495) made this pull request unmergeable. Please resolve the merge conflicts.

petrochenkov · 2019-02-23T21:19:41Z

Blocked on #58619

@steveklabnik

Cosmetic improvements to doc comments This has been factored out from rust-lang/rust#58036 to only include changes to documentation comments (throughout the rustc codebase). r? @steveklabnik Once you're happy with this, maybe we could get it through with r=1, so it doesn't constantly get invalidated? (I'm not sure this will be an issue, but just in case...) Anyway, thanks for your advice so far!

Various cosmetic improvements Related to the larger effort of rust-lang/rust#58036.

These are not legally required and are mostly noise. See: * rust-lang/rust#58036 * rust-lang/rls#1326

rust-lang/rust#58036

@steveklabnik

Cosmetic improvements to doc comments This has been factored out from rust-lang/rust#58036 to only include changes to documentation comments (throughout the rustc codebase). r? @steveklabnik Once you're happy with this, maybe we could get it through with r=1, so it doesn't constantly get invalidated? (I'm not sure this will be an issue, but just in case...) Anyway, thanks for your advice so far!

@steveklabnik

Cosmetic improvements to doc comments This has been factored out from rust-lang/rust#58036 to only include changes to documentation comments (throughout the rustc codebase). r? @steveklabnik Once you're happy with this, maybe we could get it through with r=1, so it doesn't constantly get invalidated? (I'm not sure this will be an issue, but just in case...) Anyway, thanks for your advice so far! [git filter-repo] original commit: rust-lang/rust@b244f61

@steveklabnik

Cosmetic improvements to doc comments This has been factored out from rust-lang/rust#58036 to only include changes to documentation comments (throughout the rustc codebase). r? @steveklabnik Once you're happy with this, maybe we could get it through with r=1, so it doesn't constantly get invalidated? (I'm not sure this will be an issue, but just in case...) Anyway, thanks for your advice so far!

rust-highfive assigned pnkfelix Jan 31, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 31, 2019

alexreg force-pushed the cosmetic-2 branch from 79bd23d to 80269f7 Compare January 31, 2019 23:05

This comment has been minimized.

Sign in to view

alexreg force-pushed the cosmetic-2 branch from 80269f7 to 220bdcd Compare February 1, 2019 05:17

pnkfelix changed the title ~~Massive cosmetic PR, take 2~~ [WIP] Massive cosmetic PR, take 2 Feb 1, 2019

oli-obk reviewed Feb 1, 2019

View reviewed changes

alexreg force-pushed the cosmetic-2 branch from 220bdcd to f4eb488 Compare February 1, 2019 19:14

This comment has been minimized.

Sign in to view

alexreg force-pushed the cosmetic-2 branch from f4eb488 to 27f17a6 Compare February 1, 2019 21:54

IsaacWoods reviewed Feb 1, 2019

View reviewed changes

src/build_helper/lib.rs Outdated Show resolved Hide resolved

IsaacWoods reviewed Feb 1, 2019

View reviewed changes

src/build_helper/lib.rs Outdated Show resolved Hide resolved

alexreg force-pushed the cosmetic-2 branch from 27f17a6 to 81d52ee Compare February 1, 2019 23:12

IsaacWoods reviewed Feb 1, 2019

View reviewed changes

src/liballoc/collections/btree/set.rs Outdated Show resolved Hide resolved

src/liballoc/tests/lib.rs Outdated Show resolved Hide resolved

This comment has been minimized.

Sign in to view

alexreg force-pushed the cosmetic-2 branch from 81d52ee to 768afc6 Compare February 2, 2019 01:49

memoryruins reviewed Feb 2, 2019

View reviewed changes

src/bootstrap/builder.rs Outdated Show resolved Hide resolved

src/liballoc/collections/btree/node.rs Show resolved Hide resolved

steveklabnik reviewed Feb 4, 2019

View reviewed changes

src/tools/tidy/src/style.rs Outdated Show resolved Hide resolved

alexreg force-pushed the cosmetic-2 branch from 768afc6 to 600ba92 Compare February 4, 2019 13:23

This comment has been minimized.

Sign in to view

alexreg force-pushed the cosmetic-2 branch from 600ba92 to 03c91ae Compare February 4, 2019 14:36

This comment has been minimized.

Sign in to view

Alexander Regueiro added 14 commits February 16, 2019 02:29

rustc: code formatting

fb4cf2e

rustc: imports

eb70e06

rustc: debug & error messages

bb66cef

rustc: added derives

9ea8bce

rustc: miscellaneous

cf1c49e

tests: comments

dec2e09

tests: code formatting

7274a50

tests: re-blessed

4acd315

tests: moved issue-* tests to correct location

459849e

libs: comments

c404e5a

libs: code formatting

8f8f08b

libs: debug msgs

9960acb

tools: comments

d3f4048

tools: debug & error messages

7a091d4

alexreg force-pushed the cosmetic-2 branch from 528ae6b to 7a091d4 Compare February 16, 2019 02:30

This comment has been minimized.

Sign in to view

oli-obk mentioned this pull request Feb 21, 2019

Cosmetic changes to compiler comments and docs #58619

Closed

petrochenkov added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 23, 2019

alexreg closed this Mar 1, 2019

bors added a commit to rust-lang/rust-clippy that referenced this pull request Mar 10, 2019

Auto merge of #3767 - alexreg:cosmetic-2, r=flip1995

1cdac4a

Various cosmetic improvements Related to the larger effort of rust-lang/rust#58036.

Xanewok added a commit to Xanewok/rls-vscode that referenced this pull request Mar 19, 2019

Prune copyright headers

4a467ec

These are not legally required and are mostly noise. See: * rust-lang/rust#58036 * rust-lang/rls#1326

Xanewok added a commit to rust-lang/rls that referenced this pull request Apr 20, 2019

rls-data: Remove copyright headers

ebdfab5

rust-lang/rust#58036

Xanewok added a commit to rust-lang/rls that referenced this pull request Apr 20, 2019

rls-data: Remove copyright headers

a94ab00

rust-lang/rust#58036

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Massive cosmetic PR, take 2 #58036

[WIP] Massive cosmetic PR, take 2 #58036

alexreg commented Jan 31, 2019 •

edited

Loading

rust-highfive commented Jan 31, 2019

This comment has been minimized.

pnkfelix commented Feb 1, 2019

bors commented Feb 1, 2019

alexreg commented Feb 1, 2019

oli-obk left a comment

oli-obk Feb 1, 2019

alexreg Feb 1, 2019

oli-obk Feb 1, 2019

alexreg Feb 1, 2019

This comment has been minimized.

This comment has been minimized.

bors commented Feb 3, 2019

steveklabnik Feb 4, 2019

alexreg Feb 4, 2019

alexreg Feb 4, 2019

alexreg Feb 4, 2019

steveklabnik Feb 4, 2019

alexreg Feb 4, 2019

alexreg commented Feb 4, 2019

This comment has been minimized.

This comment has been minimized.

bors commented Feb 5, 2019

This comment has been minimized.

bors commented Feb 17, 2019

petrochenkov commented Feb 23, 2019

		@@ -186,7 +186,7 @@ impl Semantics for X87DoubleExtendedS {
		/// exponent = all 1's, integer bit 0, significand 0 ("pseudoinfinity")

[WIP] Massive cosmetic PR, take 2 #58036

[WIP] Massive cosmetic PR, take 2 #58036

Conversation

alexreg commented Jan 31, 2019 • edited Loading

rust-highfive commented Jan 31, 2019

This comment has been minimized.

pnkfelix commented Feb 1, 2019

bors commented Feb 1, 2019

alexreg commented Feb 1, 2019

oli-obk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

This comment has been minimized.

bors commented Feb 3, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexreg commented Feb 4, 2019

This comment has been minimized.

This comment has been minimized.

bors commented Feb 5, 2019

This comment has been minimized.

bors commented Feb 17, 2019

petrochenkov commented Feb 23, 2019

alexreg commented Jan 31, 2019 •

edited

Loading