-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
RFC: ?
in main
#1937
RFC: ?
in main
#1937
Conversation
I like the general idea and approach. A few comments in the details:
I assume this means types implementing the Later in the RFC you propose
Why? What’s wrong with |
In various places, we've talked about a "needs-provides" feature for crates to declare items which are defined in a downstream crate. Ultimately, I do want this, and I want it to provide the underpinning for the way Anticipating this, I'd like std-less programs to stay the same, and only those linking It's fine if CC @japaric |
👍 but it should work with the |
Yes, that's what I meant. i'm not wedded to it, but I hesitate to make all I suppose we could provide Termination for
This is part of the stdlib staying out of the way of applications that want to implement particular Unix conventions. 1 is used by some programs to mean "no error but no matches found". The stdlib's Termination impls are all for errors, so it should avoid 1. (This is not an essential part of the proposal but I do think it is worthwhile.) |
text/0000-ques-in-main.md
Outdated
`Termination` don't make sense in context. | ||
|
||
There are also environments where | ||
[returning from `main` constitutes a _bug_.][divergent-main] If you |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are also environments where [returning from
main
constitutes a bug.]
It's totally fine to return from main (vanilla fn main()
) in embedded / no_std
/ bare metal / microcontroller systems; I do it all the time. Just make sure you don't return from the entry point function (e.g. reset handler) because that's UB. On bare metal systems, you are in charge of everything so this is doable. I have been using this entry point for my ARM Cortex-M programs for a while now:
/// Reset handler
#[no_mangle]
pub unsafe fn _start() -> ! {
// rustc synthesized `main` symbol. cf. `start` lang item
extern "C" {
fn main(argc: usize, argv: *const *const u8) -> isize;
}
zero_bss_section();
initialize_data_section();
main(0, ptr::null());
// Go into "reactive" mode: service interrupts as they occur and sleep when there's nothing to do
loop {
asm!("wfi" :::: "volatile");
}
}
So a microcontroller program like this is fine:
fn main() {
initialize_stuff();
}
// interrupt handler
fn on_timeout() {
LED.toggle();
}
As well as this one with a "main loop":
fn main() {
initialize_stuff();
loop {
LED.toggle();
delay_ms(500);
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not at all a microcontroller person. Divergent main
is in this RFC because it was explicitly asked for over here. Are you saying that that's not actually a useful feature?
text/0000-ques-in-main.md
Outdated
|
||
The harness for `#[test]` functions is very simple; I think it would | ||
be enough to just give `#[test]` functions the same shim that we give | ||
to `main`. The programmer would be responsible for adjusting their |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be enough to just give
#[test]
functions the same shim that we give tomain
.
This would not be compatible with no_std
test runners like utest.
Supporting #[test] fn() -> Result<_, _>
is actually simple. The test
crate already has an enum
that represents #[test] fn()
, #[bench] fn(&mut Bencher)
, etc. We just have to add a new variant that represents the Result
variant and lift the rustc
restriction that functions with #[test]
attribute must have signature fn()
. Then it would be the job of the test runner to handle the new case.
But that would be a different RFC than this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it is already captured anywhere, but I'd find this useful to be able to mark tests as "skipped at runtime".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Giving
#[test]
functions the same shim asmain
] would not be compatible withno_std
test runners like utest.
I don't follow - the whole point of the shim is to make it so external library code that expects an entry point returning ()
doesn't have to know or care about this RFC. Why wouldn't that work for utest?
... to be able to mark tests as "skipped at runtime"
I want that too (e.g. it would help with #39798), and it's a very common feature in test harnesses generally, but I'm not sure how to wedge it into this RFC. I'll think about it some more, but would you mind thinking about it too?
text/0000-ques-in-main.md
Outdated
people coming to Rust from other languages may find this _less_ | ||
surprising than the status quo. | ||
|
||
# Alternatives |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or encourage the use of templates like quickstart. That's how I start all my toy and non-toy std
apps today. Granted, that wouldn't help with doctests / documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, I'll add that to the alternatives section.
text/0000-ques-in-main.md
Outdated
if let Some(ref cause) = self.cause() { | ||
cause.write_diagnostic(progname, stream); | ||
} | ||
writeln!(stream, "{}: {}\n", progname, self.description()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That means when we use chained error the result will be reported like this, repeating the progname many times?
my_program: file not found: /foo/bar
my_program: cannot open configuration file
my_program: initialization failure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that is what Unixy programs are expected to do. The "//unspecified but not entirely unlike:" comment is meant to give wiggle room to match the expectations of other environments.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced Windows has an expectation for us to match. The following tools are all in %WINDIR%\System32
, and some aren't even consistent with themselves:
C:\Users\Guest>findstr pgjearpgje aerg jaeiajrpoerijgaerijgaeogirje
FINDSTR: Cannot open aerg
FINDSTR: Cannot open jaeiajrpoerijgaerijgaeogirje
C:\Users\Guest>find "fawerfawef" awefa awegfawergar
File not found - AWEFA
File not found - AWEGFAWERGAR
C:\Users\Guest>where /E
ERROR: Invalid argument or option - '/E'.
Type "WHERE /?" for usage help.
C:\Users\Guest>find asdfawerfawef awefawefawefw awegfawergar
FIND: Parameter format not correct
C:\Users\Guest>convert 123
Invalid drive specification.
C:\Users\Guest>taskkill /im aergaerge
ERROR: The process "aergaerge" not found.
Makes me worry that anything we pick would be widely considered surprising.
text/0000-ques-in-main.md
Outdated
|
||
``` rust | ||
trait Termination { | ||
fn write_diagnostic(&self, progname: &str, stream: &mut Write) -> (); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
std::io::Write
or core::fmt::Write
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. I was thinking std::io::Write
because that's what set_panic
takes, but depending on how much needs to be in libcore, it could wind up better the other one, I suppose...
text/0000-ques-in-main.md
Outdated
We also need to decide where the trait should live. One obvious place | ||
is in `std::process`, because that is where `exit(i32)` is; on the | ||
other hand, this is basic enough that it may make sense to put at | ||
least some of it in libcore. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Termination
should be put in libcore
, probably as a lang-item. I don't think it is a good idea for the compiler to use a hard-coded list of valid return types, where the path std::error::Error
, and worse, the OS-dependent std::process::TermStatus
need to be known by the compiler.
(Also note that the current ?
operator lowering does not refer to core::result::Result
directly, but through the trait core::ops::Carrier
.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What would you think of moving std::process::exit
into libcore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zackw The low-level entry point needs to return an i32
, there is no need to use std::process::exit
to return the exit code.
text/0000-ques-in-main.md
Outdated
Most operating systems accept only a scalar exit status, but | ||
[Plan 9][], uniquely (to my knowledge), takes an entire string (see | ||
[`exits(2)`][exits.2]). Do we care? If we care, what do we do about | ||
it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Plan 9 is not in https://forge.rust-lang.org/platform-support.html, I guess not (yet).
Plus, an integer can be easily formatted to a string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point here is that hypothetically people writing Plan 9-native programs in Rust would want full control over the string passed to exits()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think that's worth complicating the interface with. People who want full control can call a platform-specific exit function.
text/0000-ques-in-main.md
Outdated
use only `EXIT_SUCCESS` and `EXIT_FAILURE`, with one exception: | ||
|
||
* There is a type, provisionally called `TermStatus`, which is a | ||
newtype over `i32`; on Unix (but not on Windows), creating one from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows has %ERRORLEVEL% too 😢
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what you are meaning to get at with this? The (but not on windows) parenthetical is because Windows does not truncate the value passed to ExitProcess
, so it is correct to create TermStatus
quantities from arbitrary i32's on Windows. (Technically, POSIX-compliant systems are supposed to make the full value passed to _exit
available to a parent that retrieves it with waitid
instead of waitpid
, but I'm not aware of any actual Unix that meets that requirement.)
I like what you said about not having I love that For the trait: it feels weird to have two methods always called one after the other where one returns unit. Why not just On The two things above bring the trait down to just something like
(I tried to give the method a name that implies its impurity, but I suspect there's a better one.) Translating the impls from the RFC ends up something like this, which I think looks quite nice:
I'm glad you like the result owl example, but I'm still not convinced by the Result impl multiplexing back to the Termination trait. It enables a bunch of weird things, like returning |
A couple of thoughts: I think I may prefer @scottmcm's simplified traits. Ultimately, I would definitely appreciate @rust-lang/libs team feedback on "fine-tuning" the traits for ergonomics and usability, as well as the set of default impls -- this is why I tagged the RFC with (I think I still prefer supporting just The main thing I wanted to comment on, though, was the "changes to main" section. Let's start with this paragraph:
I think there is some confusion here about "input" vs "output" types (also called the "universal" vs "existential", or "any" vs "some"). Specifically the fn main() -> impl Termination { ... } Or of course we could write it, as we do today, with a concrete type: fn main() { } // -> () If you think of it in terms of traits, there is kind of a trait like: trait Main {
type Output;
fn main() -> Self::Output;
} Continuing on to to the next paragraph, I think that looking at things in terms of "input" vs "output" types is also helpful here:
I think the gist of @alexcrichton's comment on the internals thread (which, btw, it's probably worth linking to in the RFC), was that the actual starting point for execution is the fn lang_start(main: fn(), argc: isize, argv: *const *const u8) -> isize
// the actual code uses main: *const u8, but transmutes it to `fn()` later but there is no reason it can't have a generic signature: fn lang_start<T: Termination>(main: fn() -> T, argc: isize, argv: *const *const u8) -> isize The key point is that the |
Heh, yeah, |
I feel like there are two distinct features here:
The overloaded return type is handled as a bit of a fudged half-magic here. Maybe it'd be better to have a separate RFC for it? |
This RFC assumes that returning However, for real output from the program I need to have 100% control over how the error is formatted. For example, I need different formatting depending on program's verbose flag, and I don't want the program name, but an "error: " prefix, and I don't want to display all error causes, only some of them that don't expose implementation details, etc. Implementation of |
Interesting, @pornel. I agree that the output format is endlessly bikesheddable. Perhaps the only way to deal with that is to re-use an existing shed: What if an If that's the case, then it'd bring things down to just
One thing I quite like about that is that it resolves my result-multiplexing-to-Termination complaints. Having |
text/0000-ques-in-main.md
Outdated
now generic: | ||
|
||
``` rust | ||
fn<T: Termination> main() -> T { ... } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Angle brackets belong after the function name: fn main<T: Termination>() -> T { ... }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is pseudosyntax? Since the only way this proposal can work is fn main() -> T
where T: Termination
, but T
has to be a concrete type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, pseudosyntax. Also, any time you see me write something that's not quite right, please remember that I only started learning Rust this past January. (Yeah, I like jumping in at the deep end 😉)
I love this proposal. This should allow short examples to stop using However, I would find it exceptionally confusing if Rust's |
Also, one bikeshedding nit: |
I also tend to prefer @scottmcm's simplified traits, notably consuming I think I also agree with @nikomatsakis in that Finally thanks so much for mentioning doctests here! I think you proposed solution will work great (and we can use crater to confirm). I wonder if perhaps we could default to |
That wouldn't work if you want to both support error-handling (using the |
Well, to be clear, literally everything is already supported today, you can just use |
@alexcrichton |
I think there is an interesting question as to whether this kind of return value should be something formatted "nicely enough" that for simple uses it might be acceptable to present to the user, or whether it should display something very "debug-oriented", kind of analogous to an uncaught exception in a Java program (etc). The latter is sort of what I naively expected, but I think the approach the RFC takes is more the former, and it has its appeal as well. |
@nikomatsakis This loses a huge amount of value if it looks like a |
Huzzah! This RFC has been merged! Tracking issue. Thanks, all, for the long and vigorous discussion. This is going to be a great improvement for Rust. |
can the "Rendered" link in the PR description be updated so it points to the rust-lang repo instead of the personal one (which is now 404ing)? |
@radix Updated. |
…main` method or in any other method returning a non-`std::io::Result` value fails, because of reasons mentioned in rust-lang/rfcs#1937. I suggest uncommenting these parts of the examples, so that the examples are more "copy-pasteable" and show the true requirements for using them. The compilation errors I got wasn't enough to make me realize what the problem was: ``` error[E0277]: the trait bound `std::string::String: std::ops::Try` is not satisfied ``` (Thanks to the helpful people at #rust-beginners who helped me debug it; it was obvious once you knew the prerequisites for using the `?` operator.)
Question for you @rust-lang/libs folks: the Should it be |
I think process makes sense, its also where Possibly |
Personally I agree with the rationale by @withoutboats, I also thought of |
ok. |
Since the built-in supported return types may be approaching stabilization, should the RFC be amended to reflect the current implementation and the reasoning behind them? The implementation heading towards stabilization accepts E: Debug for Results, while the RFC uses E: Display. |
Guys, what prevents us from improving support of
|
@vityafx Nothing technical, but |
Simply if I have code in the main, I need a lot of ok_or things which is ugly and inconvenient |
Rendered.
Would resolve #1176. Pre-RFC discussion. See also #1718, #1859, and the internals thread about main returning
!
.Edit: Updated render link post-merge