Skip to content

Conversation

workingjubilee
Copy link
Member

C++ style guides I am aware of recommend specifically preferring = syntax for any classes with fairly obvious constructors1 that do not perform any complicated logic in their constructor. I contend that all constructors that the rustc_llvm code uses qualify. This has only become more common since C++ 17 guaranteed many cases of copy initialization elision.

The other detail is that I tried to ask another contributor with infinitely more C++ experience than me (i.e. any) what this constructor syntax was, and they thought it was a macro. I know of no other language that has adopted this same syntax. As the rustc codebase features many contributors experienced in many other languages, using a less... unique... style has many other benefits in making this code more lucid and maintainable, which is something it direly needs.

Footnotes

  1. e.g. https://abseil.io/tips/88

@rustbot
Copy link
Collaborator

rustbot commented Mar 6, 2024

r? @cuviper

rustbot has assigned @cuviper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 6, 2024
@rust-log-analyzer

This comment has been minimized.

C++ style guides I am aware of recommend specifically preferring = syntax
for any classes with fairly obvious constructors[^0] that do not perform
any complicated logic in their constructor. I contend that all constructors
that the `rustc_llvm` code uses qualify. This has only become more common
since C++ 17 guaranteed many cases of copy initialization elision.

The other detail is that I tried to ask another contributor with
infinitely more C++ experience than me (i.e. any) what this constructor
syntax was, and they thought it was a macro. I know of no other language
that has adopted this same syntax. As the rustc codebase features many
contributors experienced in many other languages, using a less...
unique... style has many other benefits in making this code more
lucid and maintainable, which is something it direly needs.

[^0]: e.g. https://abseil.io/tips/88
@cuviper
Copy link
Member

cuviper commented Mar 6, 2024

Changing to the auto Name = Type(args...); style of initialization is ok with me, especially to avoid the "most vexing parse" of the traditional constructor syntax.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Mar 6, 2024

📌 Commit 23623a0 has been approved by cuviper

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 6, 2024
bors added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2024
…iaskrgr

Rollup of 8 pull requests

Successful merges:

 - rust-lang#122015 (Add better explanation for `rustc_index::IndexVec`)
 - rust-lang#122061 (Clarify FatalErrorHandler)
 - rust-lang#122062 (Explicitly assign constructed C++ classes)
 - rust-lang#122072 (Refer to "slice" instead of "vector" in Ord and PartialOrd trait impl of slices)
 - rust-lang#122088 (Remove unnecessary fixme on new thread stack size)
 - rust-lang#122094 (Remove outdated footnote "missing-stack-probe" in platform-support)
 - rust-lang#122107 (Temporarily make allow-by-default the `non_local_definitions` lint)
 - rust-lang#122109 (compiletest: Add a `//@ needs-threads` directive)

Failed merges:

 - rust-lang#122104 (Rust is a proper name: rust → Rust)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 869529a into rust-lang:master Mar 7, 2024
@rustbot rustbot added this to the 1.78.0 milestone Mar 7, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this pull request Mar 7, 2024
Rollup merge of rust-lang#122062 - workingjubilee:initialize-my-fist, r=cuviper

Explicitly assign constructed C++ classes

C++ style guides I am aware of recommend specifically preferring = syntax for any classes with fairly obvious constructors[^0] that do not perform any complicated logic in their constructor. I contend that all constructors that the `rustc_llvm` code uses qualify. This has only become more common since C++ 17 guaranteed many cases of copy initialization elision.

The other detail is that I tried to ask another contributor with infinitely more C++ experience than me (i.e. any) what this constructor syntax was, and they thought it was a macro. I know of no other language that has adopted this same syntax. As the rustc codebase features many contributors experienced in many other languages, using a less... unique... style has many other benefits in making this code more lucid and maintainable, which is something it direly needs.

[^0]: e.g. https://abseil.io/tips/88
@workingjubilee workingjubilee deleted the initialize-my-fist branch March 7, 2024 03:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants