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

std: Remove fortune cookies from fatal runtime errors #20944

Merged
merged 1 commit into from Jan 13, 2015

Conversation

Projects
None yet
@brson
Copy link
Contributor

brson commented Jan 11, 2015

Closes #13871

@rust-highfive

This comment has been minimized.

Copy link
Collaborator

rust-highfive commented Jan 11, 2015

r? @huonw

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

@brson

This comment has been minimized.

Copy link
Owner

brson commented on 740c837 Jan 11, 2015

r+

This comment has been minimized.

Copy link

evinugur replied Jan 12, 2015

@brson These are actually quotes from 20th century American author HP Lovecraft and not fortune cookies. Either way I agree they have no right to bloat up the standard libraries

@brson brson referenced this pull request Jan 11, 2015

Closed

Closes #13871 #20035

@jfager

This comment has been minimized.

Copy link
Contributor

jfager commented Jan 11, 2015

Boooooo. (edit: In eulogy: one of the first things I worked on w/ Rust was the Matasano crypto challenges. I spent a lot of time fighting the compiler trying to learn the basics, get past the borrow checker, and work through the occasional ice, but for the most part made pretty decent progress. I finally did something that managed to get a Lovecraft quote, but it took me maybe a good 5 minutes to realize that it was a runtime error and not a sudden shift in theme in the challenge answers. Might have been my first <3 Rust moment.)

@Gankro

This comment has been minimized.

Copy link
Contributor

Gankro commented Jan 11, 2015

Instantly r+'d by the PR author... this kind of totalitarian management of the project won't stand!

😈

@MattWindsor91

This comment has been minimized.

Copy link
Contributor

MattWindsor91 commented Jan 11, 2015

I'll always remember coming back to an attempt to compile rustc, seeing You've met with a terrible fate, haven't you? and being a mixture of annoyed, confused, and (I'll admit) slightly amused xD

@csouth3

This comment has been minimized.

Copy link
Contributor

csouth3 commented Jan 11, 2015

😢 rip. I only got a Lovecraft quote once ever, but it was pretty much my favorite Rust moment ever.

@@ -144,56 +144,6 @@ pub fn abort(args: fmt::Arguments) -> ! {
let _ = write!(&mut w, "{}", args);
let msg = str::from_utf8(&w.buf[0..w.pos]).unwrap_or("aborted");
let msg = if msg.is_empty() {"aborted"} else {msg};

This comment has been minimized.

@eddyb

eddyb Jan 11, 2015

Member

All this logic is not necessary anymore.

This comment has been minimized.

@brson

brson Jan 12, 2015

Contributor

@eddyb It seems to cover real cases: the first if the buffer is not utf8 (though with fmt changing to require utf8 maybe that's impossible), and the second if the abor t message is empty. Neither of those seem to be related to the easter egg.

This comment has been minimized.

@eddyb

eddyb Jan 12, 2015

Member

Sorry, I meant the whole writing to a buffer thing. That was only done for the fortune hash. Can just write the format args to stderr now.

@tshepang

This comment has been minimized.

Copy link
Contributor

tshepang commented Jan 12, 2015

@Gankro I was surprised that @bors allows changes to be approved by their authors.

@retep998

This comment has been minimized.

Copy link
Member

retep998 commented Jan 13, 2015

I'm against this change. If anything we should be encrypting the text so that they are truly Easter eggs.

@bors

This comment has been minimized.

Copy link
Contributor

bors commented on 740c837 Jan 13, 2015

saw approval from brson
at brson@740c837

This comment has been minimized.

Copy link
Contributor

bors replied Jan 13, 2015

merging brson/rust/weve-met-with-a-terrible-fate-havent-we = 740c837 into auto

This comment has been minimized.

Copy link
Contributor

bors replied Jan 13, 2015

status: {"merge_sha": "3d5fbae33897a8340542f21b6ded913148ca9199"}

This comment has been minimized.

Copy link
Contributor

bors replied Jan 13, 2015

brson/rust/weve-met-with-a-terrible-fate-havent-we = 740c837 merged ok, testing candidate = 3d5fbae

This comment has been minimized.

Copy link
Contributor

bors replied Jan 13, 2015

fast-forwarding master to auto = 3d5fbae

This comment has been minimized.

Copy link
Contributor

bors replied Jan 13, 2015

fast-forwarding master to auto = 3d5fbae

bors added a commit that referenced this pull request Jan 13, 2015

@Gankro

This comment has been minimized.

Copy link
Contributor

Gankro commented Jan 13, 2015

@tshepang: bors also allows r=litterallyAnything; it is not built to have any particular security model other than "I listen to those with authority".

@bors bors merged commit 740c837 into rust-lang:master Jan 13, 2015

2 checks passed

continuous-integration/travis-ci The Travis CI build passed
Details
default all tests passed
@stuartpb

This comment has been minimized.

Copy link

stuartpb commented Jan 13, 2015

👏 👍 👏

@alexchandel

This comment has been minimized.

Copy link

alexchandel commented Jan 13, 2015

Why couldn't we just remove the text for non-debug builds?

@kballard

This comment has been minimized.

Copy link
Contributor

kballard commented Jan 13, 2015

I sympathize with the "2k is significant on some platforms" argument, but I am rather saddened to see this go.

I do wish it could exist only for non-debug builds. Though offhand I'm not sure how that would be possible. I'm also confused as to how this was ending up in #![no_std] builds since it apparently was part of libstd/rt/util.rs, which one would think is only present in libstd.

@liigo

This comment has been minimized.

Copy link
Contributor

liigo commented Jan 15, 2015

Nice to see useless code gone!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment