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.

Sign up for GitHub

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

Jump to bottom

Stabilize termination_trait, split out termination_trait_test #49162

Merged
merged 11 commits into from
Mar 24, 2018
Merged

Stabilize termination_trait, split out termination_trait_test #49162

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
c5c650d
Split out termination_trait_test feature gate
tmandry Mar 19, 2018
97b3bf9
Stabilize termination_trait
tmandry Mar 19, 2018
e5a55e7
Stabilize termination_trait in 1.25, not 1.26
tmandry Mar 19, 2018
be29e52
Match against friendly error message
tmandry Mar 20, 2018
5ccf3ff
diagnostics: Remove main return type errors from E0580
tmandry Mar 20, 2018
72334fe
typeck: Report main return type errors on return type span
tmandry Mar 20, 2018
1937661
Revert "Stabilize termination_trait in 1.25, not 1.26"
tmandry Mar 21, 2018
94bdeb6
termination_trait: Add () example to error message
tmandry Mar 21, 2018
2cdc7af
Use NOTE instead of error-pattern directive
tmandry Mar 21, 2018
b6934c9
termination_trait: Put examples in error help, not label
tmandry Mar 21, 2018
2b13d95
termination_trait: Make error message more helpful
tmandry Mar 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 16 additions & 19 deletions src/librustc_typeck/check/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1106,25 +1106,22 @@ fn check_fn<'a, 'gcx, 'tcx>(inherited: &'a Inherited<'a, 'gcx, 'tcx>,
}
fcx.demand_suptype(span, ret_ty, actual_return_ty);

if fcx.tcx.features().termination_trait {
// If the termination trait language item is activated, check that the main return type
// implements the termination trait.
if let Some(term_id) = fcx.tcx.lang_items().termination() {
if let Some((id, _)) = *fcx.tcx.sess.entry_fn.borrow() {
if id == fn_id {
match fcx.sess().entry_type.get() {
Some(config::EntryMain) => {
let substs = fcx.tcx.mk_substs(iter::once(Kind::from(ret_ty)));
let trait_ref = ty::TraitRef::new(term_id, substs);
let cause = traits::ObligationCause::new(
span, fn_id, ObligationCauseCode::MainFunctionType);

inherited.register_predicate(
traits::Obligation::new(
cause, param_env, trait_ref.to_predicate()));
},
_ => {},
}
// Check that the main return type implements the termination trait.
if let Some(term_id) = fcx.tcx.lang_items().termination() {
if let Some((id, _)) = *fcx.tcx.sess.entry_fn.borrow() {
if id == fn_id {
match fcx.sess().entry_type.get() {
Some(config::EntryMain) => {
let substs = fcx.tcx.mk_substs(iter::once(Kind::from(ret_ty)));
let trait_ref = ty::TraitRef::new(term_id, substs);
let cause = traits::ObligationCause::new(
span, fn_id, ObligationCauseCode::MainFunctionType);
Copy link
Contributor

Choose a reason for hiding this comment

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

the span is specified here; if you look up to line 1028 you can see it comes from the body:

rust/src/librustc_typeck/check/mod.rs

Line 1028 in 5ccf3ff

let span = body.value.span;

In general, to get a good span, you want to look to the HIR, which corresponds pretty closely to the input program. In this case, we want to look at the argument decl:

rust/src/librustc_typeck/check/mod.rs

Line 1001 in 5ccf3ff

decl: &'gcx hir::FnDecl,

The FnDecl struct is declared here:

rust/src/librustc/hir/mod.rs

Lines 1718 to 1727 in 5ccf3ff

/// Represents the header (not the body) of a function declaration
#[derive(Clone, PartialEq, Eq, RustcEncodable, RustcDecodable, Hash, Debug)]
pub struct FnDecl {
pub inputs: HirVec<P<Ty>>,
pub output: FunctionRetTy,
pub variadic: bool,
/// True if this function has an `self`, `&self` or `&mut self` receiver
/// (but not a `self: Xxx` one).
pub has_implicit_self: bool,
}

We want to get the span from the return type, for which there is a convenient method:

rust/src/librustc/hir/mod.rs

Lines 1815 to 1820 in 5ccf3ff

pub fn span(&self) -> Span {
match *self {
DefaultReturn(span) => span,
Return(ref ty) => ty.span,
}
}

So basically the span should be something like:

let return_ty_span = decl.output.span();

and use that instead of span.


inherited.register_predicate(
traits::Obligation::new(
cause, param_env, trait_ref.to_predicate()));
},
_ => {},
}
}
}
Expand Down
3 changes: 1 addition & 2 deletions src/librustc_typeck/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,8 +208,7 @@ fn check_main_fn_ty<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
}

let actual = tcx.fn_sig(main_def_id);
let expected_return_type = if tcx.lang_items().termination().is_some()
&& tcx.features().termination_trait {
let expected_return_type = if tcx.lang_items().termination().is_some() {
// we take the return type of the given main function, the real check is done
// in `check_fn`
actual.output().skip_binder()
Expand Down
2 changes: 1 addition & 1 deletion src/libstd/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@
#![feature(str_char)]
#![feature(str_internals)]
#![feature(str_utf16)]
#![feature(termination_trait)]
#![feature(test, rustc_private)]
#![feature(thread_local)]
#![feature(toowned_clone_into)]
Expand All @@ -325,6 +324,7 @@
#![cfg_attr(test, feature(update_panic_count))]
#![cfg_attr(windows, feature(used))]
#![cfg_attr(stage0, feature(never_type))]
#![cfg_attr(stage0, feature(termination_trait))]

#![default_lib_allocator]

Expand Down
5 changes: 2 additions & 3 deletions src/libsyntax/feature_gate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,9 +429,6 @@ declare_features! (
// `foo.rs` as an alternative to `foo/mod.rs`
(active, non_modrs_mods, "1.24.0", Some(44660), None),

// Termination trait in main (RFC 1937)
(active, termination_trait, "1.24.0", Some(43301), None),

// Termination trait in tests (RFC 1937)
(active, termination_trait_test, "1.24.0", Some(48854), None),

Expand Down Expand Up @@ -558,6 +555,8 @@ declare_features! (
(accepted, inclusive_range_syntax, "1.26.0", Some(28237), None),
// allow `..=` in patterns (RFC 1192)
(accepted, dotdoteq_in_patterns, "1.26.0", Some(28237), None),
// Termination trait in main (RFC 1937)
(accepted, termination_trait, "1.26.0", Some(43301), None),
);

// If you change this, please modify src/doc/unstable-book as well. You must
Expand Down
3 changes: 2 additions & 1 deletion ...le-fail/feature-gate-termination_trait.rs → ...ation-trait/termination-trait-main-i32.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

fn main() -> i32 { //~ ERROR main function has wrong type [E0580]
fn main() -> i32 {
//~^ ERROR the trait bound `i32: std::process::Termination` is not satisfied [E0277]
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, this is not your fault, but this error message seems less clear than before.

@estebank -- can we improve this readily enough with a #[rustc_on_unimplemented] annotation perhaps?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you could annotate std::process::Termination using #[rustc_on_unimplemented] to replace the error message to something appropriate. The only limitation is not knowing wether the requirement came from a return type or anywhere else, so the message will have to be more generic than it is now.

Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like there is already a #[rustc_on_unimplemented] annotation, but it is not used in the error message for some reason.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nevermind, once I cleaned my rustc build the note showed up. But since it's a label, not the primary error message, I had to use the error-message: directive to match against it.

Copy link
Contributor

@estebank estebank Mar 20, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

For what it's worth, you have further control on the rustc_on_unimplemented output by being able to set a separate message and label, as well as customize either one for specific types.

That way, after including Niko's span change you could have the following output:

error[E0277]: `main` can only return types that implement `std::process::Termination`, not `i32`
  --> src/bin/termination-trait-main-i32.rs:11:18
   |
11 | fn main() -> i32 {
   |              ^^^ can only return types that implement `std::process::Termination`
   |
   = help: the trait `std::process::Termination` is not implemented for `i32`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0277`.

Copy link
Contributor

Choose a reason for hiding this comment

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

since it's a label, not the primary error message, I had to use the error-message: directive to match against it.

You didn't need to use error-message, you could use

//~^ NOTE `main` can only return types that implement std::process::Termination, not `i32`

instead for labels.

0
}
1 change: 0 additions & 1 deletion src/test/compile-fail/rfc-1937-termination-trait/termination-trait-main-wrong-type.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![feature(termination_trait)]

fn main() -> char {
//~^ ERROR: the trait bound `char: std::process::Termination` is not satisfied
Expand Down
2 changes: 0 additions & 2 deletions src/test/compile-fail/rfc-1937-termination-trait/termination-trait-not-satisfied.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(termination_trait)]

struct ReturnType {}

fn main() -> ReturnType { //~ ERROR `ReturnType: std::process::Termination` is not satisfied
Expand Down
2 changes: 0 additions & 2 deletions src/test/run-fail/rfc-1937-termination-trait/termination-trait-for-never.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(termination_trait)]

// error-pattern:oh, dear

fn main() -> ! {
Expand Down
2 changes: 0 additions & 2 deletions src/test/run-fail/rfc-1937-termination-trait/termination-trait-for-result-box-error_err.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@
// must-compile-successfully
// failure-status: 1

#![feature(termination_trait)]

use std::io::{Error, ErrorKind};

fn main() -> Result<(), Box<Error>> {
Expand Down
2 changes: 0 additions & 2 deletions src/test/run-pass/rfc-1937-termination-trait/termination-trait-for-empty.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,4 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(termination_trait)]

fn main() {}
1 change: 0 additions & 1 deletion src/test/run-pass/rfc-1937-termination-trait/termination-trait-for-exitcode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(termination_trait)]
#![feature(process_exitcode_placeholder)]

use std::process::ExitCode;
Expand Down
2 changes: 0 additions & 2 deletions src/test/run-pass/rfc-1937-termination-trait/termination-trait-for-result-box-error_ok.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(termination_trait)]

use std::io::Error;

fn main() -> Result<(), Box<Error>> {
Expand Down
2 changes: 0 additions & 2 deletions src/test/run-pass/rfc-1937-termination-trait/termination-trait-for-result.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,6 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

#![feature(termination_trait)]

use std::io::Error;

fn main() -> Result<(), Error> {
Expand Down