Stream: project-error-handling

Topic: Nicer Assert Messages


view this post on Zulip Charles Ellis O'Riley Jr. (Oct 27 2020 at 17:10):

I have a lot of errors to work through. These are the steps I took leading up to said errors: I forked and cloned https://github.com/rust-lang/rust/blob/master/compiler/rustc_builtin_macros/src/assert.rs to my lapton(MacOS). At that point there were only 2 errors. At this point I received the guide https://rustc-dev-guide.rust-lang.org/ from DPC and went to the building and debugging rustc section. I filled in the config.toml file according to instructions and then ran ./x.py check which produced 2 errors (1) couldn't find required command: "cmake" and (2) failed to run rust/build/bootstrap/debug/bootstrap check

view this post on Zulip Joshua Nelson (Oct 27 2020 at 17:24):

@Charles Ellis O'Riley Jr. you need to install cmake

view this post on Zulip Joshua Nelson (Oct 27 2020 at 17:24):

on linux, that's sudo apt install cmake, not sure about windows

view this post on Zulip Charles Ellis O'Riley Jr. (Oct 27 2020 at 17:30):

I’m on a Mac so I assume it’s the same as linux

view this post on Zulip Joshua Nelson (Oct 27 2020 at 17:32):

on mac you'd use brew I think

view this post on Zulip Joshua Nelson (Oct 27 2020 at 17:32):

or any other package manager you have installed

view this post on Zulip Charles Ellis O'Riley Jr. (Oct 27 2020 at 17:33):

Thanks Joshua

view this post on Zulip Charles Ellis O'Riley Jr. (Oct 28 2020 at 00:19):

The build completed successfully. There are a multitude of problems popping up though. A few of which are as follows: (1) extern location for smallvec does not exist. This is emanating from the file ieee.rs where the calling is "use smallvec::{smallvec, SmallVec};" (2) #![feature(nll)] feature may not be used on the stable release channel. This file is src/lib.rs. Do I need to comment this feature out? (3) #[doc(alias = "==")] doc alias is experimental. That's it for now. Thanks.

view this post on Zulip Charles Ellis O'Riley Jr. (Oct 28 2020 at 00:42):

I researched the 2nd error and it seems that I need to run nightly Rust. I'm running the stable release so do I need to run nightyly for this project? Thanks.

view this post on Zulip Joshua Nelson (Oct 28 2020 at 01:32):

How are you building the project? x.py should set RUSTC_BOOTSTRAP which fixes those errors

view this post on Zulip Joshua Nelson (Oct 28 2020 at 01:32):

Unfortunately cargo build does not work on rustc itself

view this post on Zulip Charles Ellis O'Riley Jr. (Oct 28 2020 at 01:39):

Hi Josh. I ran ./x.py check

view this post on Zulip Charles Ellis O'Riley Jr. (Oct 28 2020 at 01:46):

After I downloaded cmake, I ran ./x.py check and received a message that it compiled successfully. I did have the bootstrap error which I now no longer have. The issues I stated are showing up in the PROBLEMS tab of VSCode.

view this post on Zulip Charles Ellis O'Riley Jr. (Oct 28 2020 at 01:48):

I did not do cargo build.

view this post on Zulip Joshua Nelson (Oct 28 2020 at 01:51):

VSCode runs cargo build. I wouldn't trust the IDE errors, x.py is unfortunately the only source of truth for whether your changes are correct.

view this post on Zulip Joshua Nelson (Oct 28 2020 at 01:51):

I think we have suggestions for making vscode go through x.py instead

view this post on Zulip Joshua Nelson (Oct 28 2020 at 01:51):

https://rustc-dev-guide.rust-lang.org/building/suggested.html?highlight=vscode#configuring-rust-analyzer-for-rustc

view this post on Zulip Charles Ellis O'Riley Jr. (Oct 28 2020 at 01:54):

Thanks Joshua. I'll take a look.

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 09 2020 at 00:34):

Status: Ran a successful x.py setup. Thanks @Joshua Nelson for your assistance. Becoming more comfortable with the layout of the compiler and reading about the AST. I just started going through the documentation for "The compiler testing framework" . No code changes yet.

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 09 2020 at 13:04):

I will not be able to be at the meeting today. My previous entry is my status. Wrapping my mind around AST. Compiler testing will be essential to know.

view this post on Zulip Jane Lusby (Nov 09 2020 at 17:49):

sounds good

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 02:21):

I reached out to the author here: https://github.com/ishitatsuyuki/rfcs/blob/generic-assert/text/0000-generic-assert.md because I felt that his examples are what I need to implement. The following is his reply:


"The original implementation idea for the assert! macro is to simply transform the expression into printing code, just like what println! does. Unfortunately, it turned out that many assert! uses today is not compatible with this, as the variables referenced don't necessarily implement Debug. As a result, a "plain text fallback" was described in the RFC to preserve backward compatibility, but that has become a huge roadblock for implementing the RFC.

Before I talk about the internals of "fallback", I'd like to slightly clarify how macros like println! works. Some macros like println! is not implemented through macro_rules!; instead, they are implemented as a compiler built-in and therefore can do much more flexible transforms to the arguments.

And to power up the assert! macro, it was also changed to be a compiler builtin. You might think that, if a macro is a compiler builtin, then can't it just "check" if a type passed is Copy or not? Unfortunately, the answer is currently no. A macro runs at the parsing stage, where the compiler has no access to type information. At this point, the only viable alternative is to do some "hack" through the highly unstable and unsound specialization, but the compiler team had some objection since they don't want to introduce more use of this unstable feature which might get removed some day."


@Charles O'Riley: My thoughts: Initially i felt I could achieve the results needed using polymorphism, i.e., creating an additional assert with the signature of assert_eq, but after all the reading, i.e., AST, and going back to the original assert code, I'm not sure. One question I do have is if assert.rs, which is currentyly in the src directory underneath rustc_builtin_macros needs to be moved/replicated to the src directory under proc_macro/bridge? Any feedback would be welcome. Thanks. :grinning:

view this post on Zulip oliver (Nov 12 2020 at 02:58):

What about using multiple dispatch? Or is that the unsound specialization
being referred to?

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 03:00):

@oliver** I can't answer that off the top of my head. I would have to research it and then still probably have questions. I'm literally taking this one step at a time.

view this post on Zulip oliver (Nov 12 2020 at 03:01):

It would be nice to know more about "the highly unstable
and unsound specialization", it leaves a lot to the imagination

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 03:05):

Good point. I'll see if I can get some feedback on that. As to my other question, does the assert need to be moved to the src directory under proc_macro/bridge?

view this post on Zulip Jane Lusby (Nov 12 2020 at 03:46):

for some reason I thought that it would be possible to make a built-in that does type checking

view this post on Zulip Jane Lusby (Nov 12 2020 at 03:47):

what about autoderef specialization?

view this post on Zulip Jane Lusby (Nov 12 2020 at 03:47):

that's not particularly fragile

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 03:48):

Feedback I just received:

Specialization is a feature that allows to override the trait implementation for a particular type. (Normally the compiler will not allow overlapping impls.) You can read about it at the tracking issue: https://github.com/rust-lang/rust/issues/31844. As you can see from the issue, the feature has gone through a long debate and yet it still isn't stabilized. In fact, the compiler team is actually going in the direction to remove it and replace it with some alternative design (e.g. always-applicable impls, http://smallcultfollowing.com/babysteps/blog/2018/02/09/maximally-minimal-specialization-always-applicable-impls/). So that's what I mean with "highly unstable and unsound specialization".

By the way, the "hack" I mentioned is implemented by @sinkuu as a prototype in https://github.com/sinkuu/rust/commit/ba00a0effd4c51f7c98247b994661a6db31ea7de. Maybe it would give you an idea how it might look like.

view this post on Zulip Jane Lusby (Nov 12 2020 at 03:48):

just need a debug case and a default case that uses the stringified repr

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 03:50):

@Jane Lusby: So, would that leave the assert where it current resides? In the src directory underneath rustc_builtin_macros

view this post on Zulip Jane Lusby (Nov 12 2020 at 03:52):

yea

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 03:52):

Great. Thanks

view this post on Zulip Jane Lusby (Nov 12 2020 at 03:52):

it shouldn't even really need to be a builtin afaik

view this post on Zulip Jane Lusby (Nov 12 2020 at 03:52):

but there's no reason to change it to a proc macro

view this post on Zulip Jane Lusby (Nov 12 2020 at 03:52):

unless compiler builtin just means internal proc macro in this case

view this post on Zulip Jane Lusby (Nov 12 2020 at 03:53):

in which case don't mind me, lol

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 03:53):

Oh, but I do mind you, lol

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 03:54):

Thanks.

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:14):

Jane Lusby said:

unless compiler builtin just means internal proc macro in this case

no, compiler builtins are handled specially: https://github.com/rust-lang/rust/blob/master/compiler/rustc_builtin_macros/src/assert.rs

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:15):

(maybe you could consider them a proc-macro that also uses rustc_private?)

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 05:40):

Is rustc_private stable? I googled it and the impression left was unstable

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:46):

no, rustc_private will never be stable

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:46):

it's the compiler internals

view this post on Zulip oliver (Nov 12 2020 at 05:50):

what about rustc_driver will that be stabilized?

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:52):

no, that's also internals

view this post on Zulip oliver (Nov 12 2020 at 05:53):

internals to what?

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:54):

the rust compiler

view this post on Zulip oliver (Nov 12 2020 at 05:54):

sure but what's the distinction?

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:54):

it's not part of the language

view this post on Zulip oliver (Nov 12 2020 at 05:54):

it's all rustc

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:54):

it's an API that's specific to a certain implementation

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:54):

no?

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:54):

rustc is a binary

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:54):

rustc_driver is a library

view this post on Zulip oliver (Nov 12 2020 at 05:54):

rustc_*

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:55):

I don't understand what you're asking.

view this post on Zulip oliver (Nov 12 2020 at 05:55):

so you can't use rustc_driver?

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:55):

you can if you opt-in to it with feature(rustc_private)

view this post on Zulip Joshua Nelson (Nov 12 2020 at 05:55):

but it's not something you should be using in day-to-day code

view this post on Zulip oliver (Nov 12 2020 at 05:56):

OIC internals are features, no I'm just curious

view this post on Zulip oliver (Nov 12 2020 at 05:57):

ignore my advances

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 05:58):

So Joshua, are you saying it’s something I might be able to use with the assert?

view this post on Zulip Joshua Nelson (Nov 12 2020 at 06:11):

sorry, I didn't mean to be so confusing

view this post on Zulip Joshua Nelson (Nov 12 2020 at 06:11):

rustc_private has nothing to do with how assert! is currently implemented

view this post on Zulip Joshua Nelson (Nov 12 2020 at 06:11):

I was speculating that in an alternative universe, you could imagine it being implemented as a proc_macro that opted in with rustc_private

view this post on Zulip Joshua Nelson (Nov 12 2020 at 06:12):

forget I said anything

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 12 2020 at 06:13):

Thanks. I can take that option off my plate:+1:

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 16 2020 at 22:35):

I noticed the rustc_ast crate being used. When I researched the crate, there was a note stipulating that "This API is completely unstable and subject to change." I'm wondering if someone has insight on this/is it really a matter of concern? Thanks.

view this post on Zulip Jane Lusby (Nov 16 2020 at 22:36):

It's not a matter of concern because the code you're adding is part of the same repo

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 16 2020 at 22:41):

Thanks

view this post on Zulip Charles Ellis O'Riley Jr. (Nov 21 2020 at 21:16):

I want to run a test but I'm not sure if it should be a unit test or just ./x.py assert. This will be for https://github.com/rust-lang/rust/blob/master/compiler/rustc_builtin_macros/src/assert.rs

My next question is "do I write my test code/dummy code within assert.rs? If not then where. Thanks.

view this post on Zulip Charles Ellis O'Riley Jr. (Dec 05 2020 at 21:47):

Can someone tell me where in this code the boolean expression is evaluated to true for assert!? The code can be found at https://github.com/rust-lang/rust/blob/master/compiler/rustc_builtin_macros/src/assert.rs I've been reading the Module rustc_ast::ast document to understand the language AST but I need to see how that one piece is evaluated in order to have an idea where I need to insert my code for additional functionality, i.e. assert_eq! If I'm off base here just let me know but I think I'm taking the right approach. Thanks.

view this post on Zulip Noah Lev (Dec 05 2020 at 22:34):

That code doesn't evaluate the condition; it expands the call to the macro to some other code. It can't evaluate the condition there because that code is run at compile-time.

view this post on Zulip Noah Lev (Dec 05 2020 at 22:34):

If you look at the output from rustc +nightly -Z unpretty=expanded -, you'll see that

fn main() {
assert!(true);
}

expands to

#![feature(prelude_import)]
#![no_std]
#[prelude_import]
use ::std::prelude::v1::*;
#[macro_use]
extern crate std;
fn main() { if !true { ::core::panicking::panic("assertion failed: true") }; }

view this post on Zulip Noah Lev (Dec 05 2020 at 22:35):

That if expression is generated here:

    let if_expr =
        cx.expr_if(sp, cx.expr(sp, ExprKind::Unary(UnOp::Not, cond_expr)), panic_call, None);

view this post on Zulip Charles Ellis O'Riley Jr. (Dec 05 2020 at 22:45):

(deleted)

view this post on Zulip Charles Ellis O'Riley Jr. (Dec 05 2020 at 22:49):

It seems I need to modify where the if expression is generated for assert!

view this post on Zulip Charles Ellis O'Riley Jr. (Dec 05 2020 at 23:15):

I’m making assumptions that apparently aren’t based on anything concrete. I need to follow the code from start to finish. I just hit a bump with the AST. @Camelid Thanks for the feedback 🙏

view this post on Zulip Charles Ellis O'Riley Jr. (Jan 13 2021 at 09:21):

This is the code I'm looking at: https://github.com/rust-lang/rust/blob/master/compiler/rustc_builtin_macros/src/assert.rs

My task is to add the functionality of assert_eq to assert such as follows:
macro_rules! assert_eq {
($left:expr, $right:expr) => ({
match (&$left, &$right) {
(left_val, right_val) => {
if !(*left_val == *right_val) {
panic!(r#"assertion failed: (left == right)
left: {:?},
right: {:?}"#, left_val, right_val)
}
}
}
});
}

Where I'm at:
I have read up on AST and understand that that is how the compiler represents source code and that this is done within 2 steps, lexing and parsing. It's the parsing that takes the tokens and converts that into an AST, which, I assume is what I'm looking at at the link https://github.com/rust-lang/rust/blob/master/compiler/rustc_builtin_macros/src/assert.rs . I've ran the code: rustc +nightly -Z unpretty=expanded and viewed the expanded output.

Questions:

  1. What is the code doing at line 60 of the link I provided?
  2. Will I need to add another item to struct Assert, i.e., cond_expr2, at line 65 of the link?
  3. Is there a visual debug process I can use to step through the code from ASCI format to AST? There probably isn't but I thought I'd ask.

Thanks in advance for the assistance. I'll be back on line by noon CST.

view this post on Zulip Joshua Nelson (Jan 13 2021 at 13:16):

  1. I'm not sure what you mean by visual debug process, but you can use gdb on the rust compiler if you enable debug symbols. You'd set debuginfo = 2 in config.toml (but be warned that takes a lot longer than debuginfo = 1).

view this post on Zulip Charles Ellis O'Riley Jr. (Jan 13 2021 at 16:13):

Thanks @Joshua Nelson. My first programming language was PowerBuilder which allowed one to see all the properties and their values, local, global, and shared, as you step through the code. Again, thanks.

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:35):

@Charles Ellis O'Riley Jr. 1. that seems to be constructing an if statement to add to the AST

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:36):

  1. I don't think so, you're probably going to want to modify how it generates panic_call

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:37):

rn what its doing is taking an assert and turning it into an if + a panic

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:37):

assert (bool) => if bool { panic }

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:39):

this line right here is probably the most relevant one

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:39):

https://github.com/rust-lang/rust/blob/master/compiler/rustc_builtin_macros/src/assert.rs#L55

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:39):

where it takes the expression of the assert and turns it into a string to print in the panic

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:40):

instead of just stringifying the expression you're going to want to print the values on the left and right side of the boolean expr in the assert, if applicable

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:40):

@Charles Ellis O'Riley Jr. would it help if I wrote up some more detailed instructions on the steps to take?

view this post on Zulip Charles Ellis O'Riley Jr. (Jan 13 2021 at 20:43):

@Jane Lusby, for #2, I kind of thought that’s what was going on so I assume I’ll need to add on to it. For #1, I’ll have to flesh out what you’re saying. Perhaps I’ll get a better understanding once I set up debug. Thanks for the reply.

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:44):

np, to clarify on 1.

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:45):

what I mean is, when rustc compiles code you know how it loads the source file, tokenizes it, and then makes the ast (not sure this is exactly correct but it should be close enough)

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:45):

where the AST is just rust structs that represent rust source

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:45):

so instead of if bool { body } you have something like

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:45):

struct IfStatement {
    predicate: BoolExpr,
    body: Block,
}

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:46):

not real AST but hopefully this makes it clear what it means

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:46):

the let if_expr = cx.expr_if(sp, cx.expr(sp, ExprKind::Unary(UnOp::Not, cond_expr)), panic_call, None);

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:46):

is creating one of those if expression structs

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:46):

which it's then substituting in place of the assert! macro expression in the original source

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:47):

or something like that

view this post on Zulip Charles Ellis O'Riley Jr. (Jan 13 2021 at 20:53):

Gotcha. that makes sense. So, at line 65, will I need to add another cond_expr such as cond_expr2? Not sure how that would work. I'm just thinking ahead.

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:56):

I don't think you'll need a new conditional expression

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:58):

I think the first step @Charles Ellis O'Riley Jr. is that you'll want to create the debug code for printing the condition

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:58):

and just prove that that works

view this post on Zulip Jane Lusby (Jan 13 2021 at 20:58):

let me do a little digging

view this post on Zulip Charles Ellis O'Riley Jr. (Jan 13 2021 at 20:58):

thanks

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:44):

okay @Charles Ellis O'Riley Jr. it turns out this code was already implemented in https://github.com/de-vri-es/assert2-rs

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:45):

cc @Maarten de Vries

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:45):

this bit is the important one

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:45):

https://github.com/de-vri-es/assert2-rs/blob/main/src/maybe_debug.rs

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:46):

where he used autoderef specialization to create the fallback formatting

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:48):

and https://github.com/de-vri-es/assert2-rs/blob/main/assert2-macros/src/lib.rs#L60-L85 is where it gets used

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:48):

to create the better panic

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:48):

so you're gonna need to create something like this to replace the current panic_call on line 65

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:50):

important to note though, assert2-rs uses syn + quote

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:50):

since it's an external proc macro

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:51):

so the way you actually create a type like this will be different, using the equivalent rustc APIs

view this post on Zulip Charles Ellis O'Riley Jr. (Jan 13 2021 at 21:51):

@Jane Lusby Line 65 of assert.rs?

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:51):

yes

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:52):

the expr_if line we were talking about before

view this post on Zulip Charles Ellis O'Riley Jr. (Jan 13 2021 at 21:53):

@Jane Lusby This is what's at line 65

struct Assert {
cond_expr: P<ast::Expr>,
custom_message: Option<TokenStream>,
}

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:53):

sorry i meant 61

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:53):

https://github.com/rust-lang/rust/blob/master/compiler/rustc_builtin_macros/src/assert.rs

view this post on Zulip Jane Lusby (Jan 13 2021 at 21:53):

https://github.com/rust-lang/rust/blob/master/compiler/rustc_builtin_macros/src/assert.rs#L61

view this post on Zulip Charles Ellis O'Riley Jr. (Jan 13 2021 at 21:54):

Ok. Let me look at it. Thanks.


Last updated: Jan 29 2022 at 11:01 UTC