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

efficacious `#[must_use]` methods for nightly, fix warnings #225

Merged
merged 1 commit into from Sep 13, 2017

Conversation

@zackmdavis
Copy link
Contributor

zackmdavis commented Sep 1, 2017

137660d annotated many methods with #[must_use], but despite not
being rejected by the compiler, this is actually a no-op (as of Rust
1.20). #[must_use] for functions is now available on nightly
Rust (since rust-lang/rust@3645b06), under a feature-gate (since
rust-lang/rust@8492ad2 and 35c44941), with a warning if #[must_use]
appears on a function or method without the feature being enabled—a
warning that regrettably added a lot of noise to Euclid's build, as
reported in rust-lang/rust#44213. But with judicious use of cfg_attr,
Euclid can make use of this functionality if our "unstable" feature is
enabled, and shut up the warnings in any case.

must_euclid

(SHA in the screenshot doesn't match because I had to amend the commit message)


This change is Reviewable

137660d annotated many methods with `#[must_use]`, but despite not
being rejected by the compiler, this is actually a no-op (as of Rust
1.20). `#[must_use]` for functions is now available on nightly
Rust (since rust-lang/rust@3645b06), under a feature-gate (since
rust-lang/rust@8492ad2 and 35c44941), with a warning if `#[must_use]`
appears on a function or method without the feature being enabled—a
warning that regrettably added a lot of noise to Euclid's build, as
reported in rust-lang/rust#44213. But with judicious use of `cfg_attr`,
Euclid can make use of this functionality if our "unstable" feature is
enabled, and shut up the warnings in any case.
@nox
Copy link
Member

nox commented Sep 13, 2017

@bors-servo r+

Thanks for your contribution.

@bors-servo
Copy link
Contributor

bors-servo commented Sep 13, 2017

📌 Commit 8f1a29c has been approved by nox

@bors-servo
Copy link
Contributor

bors-servo commented Sep 13, 2017

Testing commit 8f1a29c with merge 921f32c...

bors-servo added a commit that referenced this pull request Sep 13, 2017
efficacious `#[must_use]` methods for nightly, fix warnings

137660d annotated many methods with `#[must_use]`, but despite not
being rejected by the compiler, this is actually a no-op (as of Rust
1.20). `#[must_use]` for functions is now available on nightly
Rust (since rust-lang/rust@3645b06), under a feature-gate (since
rust-lang/rust@8492ad2 and 35c44941), with a warning if `#[must_use]`
appears on a function or method without the feature being enabled—a
warning that regrettably added a lot of noise to Euclid's build, as
reported in rust-lang/rust#44213. But with judicious use of [`cfg_attr`](https://doc.rust-lang.org/1.19.0/book/first-edition/conditional-compilation.html#cfg_attr),
Euclid can make use of this functionality if our "unstable" feature is
enabled, and shut up the warnings in any case.

![must_euclid](https://user-images.githubusercontent.com/1076988/29959879-3f527186-8eae-11e7-915b-357f39e4be17.png)

(SHA in the screenshot doesn't match because I had to amend the commit message)

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/euclid/225)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Sep 13, 2017

☀️ Test successful - status-travis
Approved by: nox
Pushing 921f32c to master...

@bors-servo bors-servo merged commit 8f1a29c into servo:master Sep 13, 2017
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.