Revise `lang_item` demo to something unrelated to `Box` impl #22499

pnkfelix · 2015-02-18T20:58:38Z

Revise lang_item demo to something unrelated to Box impl

Precursor for landing overloaded-box, since that will decouple the box syntax from the exchange heap (and should eliminate the use of the two aforementioned lang items).

Instead, the new demonstration program shows a definition of the panic_bounds_check lang item.

(We do not have that many procedural lang-items to choose from, which is a good sign for our efforts to decouple the compiler from the runtime!)

Precursor for overloaded-box and placement-in; see Issue #22181.

rust-highfive · 2015-02-18T20:58:49Z

r? @brson

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

pnkfelix · 2015-02-18T20:59:39Z

r? @steveklabnik

eddyb · 2015-02-18T21:01:16Z

The diff looks best in split view mode.

pnkfelix · 2015-02-18T21:01:40Z

(note that I'm no longer trying to land #22181 for alpha2, so this can wait until after the release.)

pnkfelix · 2015-02-20T00:16:08Z

odd, why doesn't homu list @steveklabnik as being assigned to this PR ?

pnkfelix · 2015-02-22T15:53:07Z

@steveklabnik note that if you have more aggressive plans for this part of the documentation, and do not wish to review this change, I can take the alternative approach of simply removing the lang_item section from this document. that would be equally effective in unblocking #22181

steveklabnik · 2015-02-23T00:50:46Z

@pnkfelix no, this looks good. I do have other plans for these docs, but I don't mind updating in the meantime. thanks!

steveklabnik · 2015-02-23T00:51:05Z

@bors: r+ 2b7f7f2 rollup

…loc-and-free, r=steveklabnik Revise `lang_item` demo to something unrelated to `Box` impl Precursor for landing overloaded-`box`, since that will decouple the `box` syntax from the exchange heap (and should eliminate the use of the two aforementioned lang items). Instead, the new demonstration program shows a definition of the `panic_bounds_check` lang item. (We do not have that many procedural lang-items to choose from, which is a good sign for our efforts to decouple the compiler from the runtime!) ---- Precursor for overloaded-`box` and placement-`in`; see Issue rust-lang#22181.

Manishearth · 2015-02-23T17:53:30Z

Needs Manishearth@a283a40 to pass doctests.

I'm also getting this failure with the change in the rollup:

$ make check-stage1-doc-trpl-unsafe
...
rustc: ../../../../src/llvm/lib/IR/Instructions.cpp:281: void llvm::CallInst::init(llvm::Value*, llvm::ArrayRef<llvm::Value*>, const llvm::Twine&): Assertion `(i >= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with a bad signature!"' failed.
Aborted (core dumped)

I can't repro it on nightly, so I might take this PR out of the rollup since I'm bad at debugging llvm. Sorry :(

pnkfelix · 2015-02-24T12:41:43Z

@bors: r-

(This is probably going to need changes along the lines that @Manishearth outlined above.)

steveklabnik · 2015-03-02T01:53:24Z

So, what's up with this PR?

pnkfelix · 2015-03-02T07:29:19Z

@steveklabnik I haven't had time to investigate it too much; the last time I looked at it, I think I had thought there might be a different lang item I could override that would not require so much surrounding infrastructure (since that was indeed one of the nice things about this example as originally composed: how short it is)!

Manishearth · 2015-03-03T12:48:30Z

Aside: Having a macro that brings in lang items would be interesting and might make this easier for people trying it out. Eg insert_lang!(Sized, Copy, Send). Might want to have two versions of this, one that inserts the actual lang item, and one which inserts a barebones version (eg an empty eh_personality, etc) with little to no dependencies.

pnkfelix · 2015-03-03T13:56:24Z

@Manishearth better still would probably be to just make rustc handle omitted lang items more robustly. I.e. there are already some cases where if you omit the lang item, it just does "the right thing." (An example would be to treat omission of the sized lang item as just meaning that all bounds are implicitly sized, as usual, and thus one cannot express an unsized bound in that context.)

Manishearth · 2015-03-03T15:41:08Z

I'm not very fond of implicit "hidden behavior" with lang items myself. If someone is using lang items, they're probably writing something low level.

Also, which would we pick? Eg for panic_fmt, should we fill the default unwinding support, or a more simplistic one, or an empty method?

Rust generally tries to be explicit over implicit, I'd prefer to continue that philosophy to lang items if we have to add any sort of auto-lang-item feature.

pnkfelix · 2015-03-03T17:27:10Z

@Manishearth I guess I was mostly thinking of the various trait lang items (e.g. Copy, Sized, etc), whose presence as a lang item is largely ... to integrate with the module hierarchy and name resolution process?

I agree that for procedural lang items, it make sense to require them.

Manishearth · 2015-03-03T17:27:55Z

Agreed on that.

pnkfelix · 2015-03-03T17:55:28Z

I've rewritten the program again, so I think it needs another review.

r? @steveklabnik

Manishearth · 2015-03-03T19:26:07Z

src/doc/trpl/unsafe.md

-`Box` then there is no need to define functions for `exchange_malloc`
-and `exchange_free`. `rustc` will emit an error when an item is needed
-but not found in the current crate or any that it depends on.
+array indexing `a[i]` then there is no need to define a function for


Nit: Might need to be "array indexing (a[i])"?

Yeah, this would make it consistent with evertying else

steveklabnik · 2015-03-03T22:06:56Z

src/doc/trpl/unsafe.md

-and one for deallocation. A freestanding program that uses the `Box`
-sugar for dynamic allocations via `malloc` and `free`:
+For example, there are lang items related to the implementation of
+string slices (`&str`); one of these is `str_eq`, which implements the


We already talk about this all the time, so you can just say 'string slices' or &str, rather than both. I guess it doesn't hurt though.

steveklabnik · 2015-03-03T22:08:47Z

@pnkfelix I like this example, it's a good one. One or two very minor nits, and then r=me

…ed to Box. Precursor for landing overloaded-`box`, since that will decouple the `box` syntax from the exchange heap (and in fact will eliminate the use of the two aforementioned lang items). Instead, the new demonstration program shows a definition of the `str_eq` lang item. (We do not have that many procedural lang-items to choose from, which is a good sign for our efforts to decouple the compiler from the runtime!) (This previously used a demo of `panic_bounds_check`, but a `str_eq` demonstration is both easier to code and arguably a more interesting aspect of the language to discuss.)

pnkfelix · 2015-03-04T14:56:06Z

@bors r=steveklabnik e5fab33 rollup

pnkfelix · 2015-03-05T15:34:34Z

@bors r-

(I think the arithmetic overflow work has introduced a dependence of + on the panic lang item, which was not reflected in this version of the code. I keep making the mistake of forgotting how late in the build cycle the doc tests are run. :( )

Manishearth · 2015-03-05T15:54:54Z

I believe you can just run rustdoc --test on the md file with a stage1 rustdoc and it will work. Not sure.

-----Original Message-----
From: "Felix S Klock II" notifications@github.com
Sent: ‎3/‎5/‎2015 9:04 PM
To: "rust-lang/rust" rust@noreply.github.com
Cc: "Manish Goregaokar" manishsmail@gmail.com
Subject: Re: [rust] Revise lang_item demo to something unrelated to Boximpl (#22499)

@bors r-
(I think the arithmetic overflow work has introduce a dependence of + on the panic lang item, which was not reflected in this version of the code. I keep making the mistake of forgotting how late in the build cycle the doc tests are run. :( )
—
Reply to this email directly or view it on GitHub.

eddyb · 2015-03-05T16:09:35Z

@Manishearth There should be a target for doc tests, even at stage1, but it might be one per-crate or something unmanageable.

pnkfelix · 2015-03-06T00:22:30Z

(closing pull request; I'm going to fold this commit in with #22086 because I actually put that through a full make check-stage1 && make check-stage2 each time before I attempt to land it.)

pnkfelix · 2015-06-08T15:59:15Z

Argh, something has changed in the last three months that broke the new demo. :(

In particular, when I try to compile it (playpen), I get:

<anon>:11:12: 11:18 error: binary operation `!=` cannot be applied to type `usize` [E0369]
<anon>:11         if s1_len != s2_len { return false; }
                     ^~~~~~
<anon>:14:15: 14:16 error: binary operation `<` cannot be applied to type `_` [E0369]
<anon>:14         while i < s1_len {
                        ^
<anon>:21:16: 21:18 error: binary operation `!=` cannot be applied to type `u8` [E0369]
<anon>:21             if b1 != b2 { return false; }
                         ^~
<anon>:30:12: 30:21 error: binary operation `<=` cannot be applied to type `u8` [E0369]
<anon>:30         if 'A' as u8 <= b && b <= 'Z' as u8 {
                     ^~~~~~~~~
<anon>:30:30: 30:31 error: binary operation `<=` cannot be applied to type `u8` [E0369]
<anon>:30         if 'A' as u8 <= b && b <= 'Z' as u8 {
                                       ^
<anon>:31:13: 31:14 error: binary operation `-` cannot be applied to type `u8` [E0369]
<anon>:31             b - ('A' as u8) + ('a' as u8)
                      ^
<anon>:71:12: 71:29 error: binary operation `!=` cannot be applied to type `u8` [E0369]
<anon>:71         if str::last_byte(s) != '\0' as u8 {
                     ^~~~~~~~~~~~~~~~~
<anon>:81:12: 81:29 error: binary operation `!=` cannot be applied to type `u8` [E0369]
<anon>:81         if str::last_byte(s) != '\0' as u8 {
                     ^~~~~~~~~~~~~~~~~
<anon>:107:30: 107:33 error: binary operation `-` cannot be applied to type `usize` [E0369]
<anon>:107             at_offset(bytes, len-1)
                                        ^~~
<anon>:113:12: 113:22 error: binary operation `+` cannot be applied to type `usize` [E0369]
<anon>:113         *((p as usize + i) as *const u8)
                      ^~~~~~~~~~
error: aborting due to 10 previous errors
playpen: application terminated with error code 101

(Its possible that fixing this is just a matter of defining and importing the appropriate comparison traits. But if that is the case, it is ... depressing...)

Manishearth · 2015-06-08T16:01:11Z

Perhaps we're missing the PartialOrd &c impls?

Precursor for landing overloaded-box, since that will decouple the box syntax from the exchange heap (and should eliminate the use of the `malloc` and `free` lang items). (This is a simplified approach to PR rust-lang#22499; note that I have once again changes which lang item to use for the illustration.)

rust-highfive assigned brson Feb 18, 2015

rust-highfive assigned steveklabnik and unassigned brson Feb 18, 2015

bombless mentioned this pull request Feb 23, 2015

Rollup of 20 pull requests #22703

Closed

pnkfelix force-pushed the purge-demo-of-exchange-malloc-and-free branch from 2b7f7f2 to 3b22ed8 Compare March 3, 2015 17:48

Manishearth reviewed Mar 3, 2015
View reviewed changes

pnkfelix mentioned this pull request Mar 3, 2015

overloaded-box protocol changes #22086

Closed

steveklabnik reviewed Mar 3, 2015
View reviewed changes

pnkfelix force-pushed the purge-demo-of-exchange-malloc-and-free branch from 3b22ed8 to e5fab33 Compare March 4, 2015 14:54

pnkfelix closed this Mar 6, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Revise `lang_item` demo to something unrelated to `Box` impl #22499

Revise `lang_item` demo to something unrelated to `Box` impl #22499

pnkfelix commented Feb 18, 2015

rust-highfive commented Feb 18, 2015

pnkfelix commented Feb 18, 2015

eddyb commented Feb 18, 2015

pnkfelix commented Feb 18, 2015

pnkfelix commented Feb 20, 2015

pnkfelix commented Feb 22, 2015

steveklabnik commented Feb 23, 2015

steveklabnik commented Feb 23, 2015

Manishearth commented Feb 23, 2015

pnkfelix commented Feb 24, 2015

steveklabnik commented Mar 2, 2015

pnkfelix commented Mar 2, 2015

Manishearth commented Mar 3, 2015

pnkfelix commented Mar 3, 2015

Manishearth commented Mar 3, 2015

pnkfelix commented Mar 3, 2015

Manishearth commented Mar 3, 2015

pnkfelix commented Mar 3, 2015

Manishearth Mar 3, 2015

steveklabnik Mar 3, 2015

steveklabnik Mar 3, 2015

steveklabnik commented Mar 3, 2015

pnkfelix commented Mar 4, 2015

pnkfelix commented Mar 5, 2015

Manishearth commented Mar 5, 2015

eddyb commented Mar 5, 2015

pnkfelix commented Mar 6, 2015

pnkfelix commented Jun 8, 2015

Manishearth commented Jun 8, 2015

Revise lang_item demo to something unrelated to Box impl #22499

Revise lang_item demo to something unrelated to Box impl #22499

Conversation

pnkfelix commented Feb 18, 2015

rust-highfive commented Feb 18, 2015

pnkfelix commented Feb 18, 2015

eddyb commented Feb 18, 2015

pnkfelix commented Feb 18, 2015

pnkfelix commented Feb 20, 2015

pnkfelix commented Feb 22, 2015

steveklabnik commented Feb 23, 2015

steveklabnik commented Feb 23, 2015

Manishearth commented Feb 23, 2015

pnkfelix commented Feb 24, 2015

steveklabnik commented Mar 2, 2015

pnkfelix commented Mar 2, 2015

Manishearth commented Mar 3, 2015

pnkfelix commented Mar 3, 2015

Manishearth commented Mar 3, 2015

pnkfelix commented Mar 3, 2015

Manishearth commented Mar 3, 2015

pnkfelix commented Mar 3, 2015

Manishearth Mar 3, 2015

Choose a reason for hiding this comment

steveklabnik Mar 3, 2015

Choose a reason for hiding this comment

steveklabnik Mar 3, 2015

Choose a reason for hiding this comment

steveklabnik commented Mar 3, 2015

pnkfelix commented Mar 4, 2015

pnkfelix commented Mar 5, 2015

Manishearth commented Mar 5, 2015

eddyb commented Mar 5, 2015

pnkfelix commented Mar 6, 2015

pnkfelix commented Jun 8, 2015

Manishearth commented Jun 8, 2015

Revise `lang_item` demo to something unrelated to `Box` impl #22499

Revise `lang_item` demo to something unrelated to `Box` impl #22499