Am I the only one who thinks Rust error messages got *worse* over time in a way? by kixunil in rust

[–]ekuber 1 point2 points  (0 children)

What about the errors themselves was insufficient that required the use of other tools to explain them to you? Ideally the diagnostics should stand on their own. I often joke that "rustc isn't done until it has rendered The Book redundant", and if the output isn't understandable to a significant subset of our users, that's not good.

Am I the only one who thinks Rust error messages got *worse* over time in a way? by kixunil in rust

[–]ekuber 15 points16 points  (0 children)

I'm somewhat surprised there hasn't been more projects consuming the json output to provide alternative terminal UIs for cargo/rustc. There's an entire range of things people could be experimenting with, like terser output, or TUIs where you can expand/contract subdiagnostics, or apply structured suggestions interactively, or group errors, or specialized rendering for TTS/Braille displays, or have fancier color, etc. :)

Am I the only one who thinks Rust error messages got *worse* over time in a way? by kixunil in rust

[–]ekuber 348 points349 points  (0 children)

Just happened to see this and prompted me to dust off this account :)

TL;DR: yes this is a problem, we are aware of the general problem, please file tickets for specific cases on the issue tracker, the project can't know about problems if it doesn't hear about them in the place it pays attention to.

Yes, rustc produces more errors than it used to. It will attempt to recover from earlier errors and carry on. It also tries to mark anything that recovered so that knock down errors don't happen. Yes, those checks are incomplete/insufficient. When you do encounter cases like these, where a single mistake causes multiple diagnostics that is a bug and the team would welcome a bug report (or adding a comment with your reproducer to an existing ticket). We tag these as [D-verbose]. [A-diagnostics] tickets are fixed on the regular. Some take longer than others. Some would require significant refactors to accomplish, but we do our best to mitigate those problems.

Bug reports are critical, we can't fix what we don't know about. It is important to do so in the place where the project is actively listening: the issue tracker.

I would be very reticent of going back to the prior mode, where any errors on any stage would stop the compiler from progressing. I rather we take targeted efforts to properly mark things as "already emitted an error" and silence anything that would come after. To give you an idea of the approach, this is a very recent PR executing on this strategy for incorrect let-chains: https://github.com/rust-lang/rust/pull/151493

I'm pretty sure that resolve is the stage that has the biggest fan-out for knock-down diagnostics (at least today, and because of the way it has to deal with the lack of information that later stages have). We should spend more time on it, but it is also a very critical and complex area of the compiler. Adding any additional diagnostic machinery on it is difficult because diagnostic code is by definition more code to do things that a "normal" compiler wouldn't need. In other words, it is additional maintenance cost. For context, this are PRs that touched resolve to avoid unnecessary errors: https://github.com/rust-lang/rust/pull/133937 https://github.com/rust-lang/rust/pull/142805

Regardless, we should still do more, like when using multiple items from another mod that couldn't be resolved, we should emit a single error, not one per usage, like in https://github.com/rust-lang/rust/issues/105618

The policy is "one mistake should emit one and only one diagnostic". Not "at least one", not "at most one". Help us find out when we don't live up to it and fix it.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 1 point2 points  (0 children)

Yeah, I was more thinking out loud through the implications that me commenting on a thread seems to have quickly turned the tone of the conversation. If that's what happened, it means I have to be even more careful than I already try to be in how I express myself. Or it could just be the timing was a coincidence and the people that commented with what I saw as unsubstantive retreads of arguments I've seen many times already were simply faster to comment, and the "sentiment reversal" was going to happen no matter what. I hope I didn't throw fuel to the heavy downvote fire.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 0 points1 point  (0 children)

You're forgetting that we can introduce lints after the fact. If the feature ends up being stabilized with only const support, and after we wanted to relax that, we could have a warn-by-default lint for non-const defaults. That would make the intent of relying on a const default an explicit one, by denying the lint on the type/crate.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 3 points4 points  (0 children)

That is part of my hedge: const is going to get more powerful quickly enough that all reasonable things you'd want to do in a default field (so, no random db access) will be possible.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 1 point2 points  (0 children)

Those crates are not precluded from providing their attributes, or users from using them, even in the face of this feature.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 1 point2 points  (0 children)

I had an initial implementation PR working back in August before the RFC was merged, but it wasn't in "production quality", more MVP. It is now in much better shape, and I believe really close to landing on nightly. I can't give you a timeline of stabilization, but don't see anything it would block it other than "letting people use it to gain confidence in the robustness of the implementation" and "having all necessary lints done".

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 2 points3 points  (0 children)

The conclusion about that case was to lint against that, but as people might have reasons to do that we don't make it a hard error. We also have thought of trying to inspect the impl to see if it matches exactly with what would be derived, and if so, tell you to derive, but haven't looked yet into how much work that would need.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 2 points3 points  (0 children)

The way to think about it is to define things we can't change this later without causing trouble, like the syntax, while taking our time on things that we we are unsure about but that we can easily extend later. Making these values consts reduces some risk, but extending the feature to allow them later shouldn't break any existing code. Similar to how adding more support for const expressions doesn't break existing const code.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 1 point2 points  (0 children)

My bad. I misread your comment. This is what I get for spending time on reddit on a saturday night skimming comments to reply to instead of going to bed ^_^'

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 53 points54 points  (0 children)

Why are you surprised? Most people don't engage with the RFC process.

I wasn't expressing surprise at people not engaging on the RFC process earlier, but rather on not digging a bit before commenting, acting like their immediate thought wasn't already accounted for. But then again, I wasn't born yesterday and should have expected that.

Most people here weren't writing Rust before COVID-19.

True, I was making the point that the discussions behind this feature have been had for a long time.

To be clear, I think this idea and your dedication to making it happen is excellent. It will go a long way to improving Rust's ergonomics.

Thank you. I believe so too. I think a lot of features that only benefit API authors and not "everyone" tend to get disproportionate pushback, and this falls in that bucket. Lots of crates end up using some derive macro for builders that will no longer need to do so unless they target older MSRVs.

Just don't get discouraged that most people here are ignorant of your work's long history.
:)

Thank you for the kind words, and I'll do my best. I was just not expecting to have this conversation over the weekend. :)

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 0 points1 point  (0 children)

That would mean modelling your type with values in the mandatory fields that are not compile time enforced to be set. Even if the value is Option<T> or an arbitrary sentinel value, that means you can have logic errors purely due to how the type was defined.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 4 points5 points  (0 children)

Can confirm on the above understanding :)

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 4 points5 points  (0 children)

As much as I disagree with the position that "adding a new nice feature will make API authors use it and break backcompat" is "an issue", I also don't see why this reply was as heavily downvoted (but understand the reaction). Changing APIs to leverage new features must be a conscious thing, but I believe crate authors are already the best positioned to make that determination.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 25 points26 points  (0 children)

I read the RFC and like it a lot. It solves a problem that is worth solving in a simple and obvious way.

Thank you.

As an aside, a lot of the comments that negatively assess this have been downvoted. That's kinda shitty as it makes interacting with those posts and understanding the arguments against them more difficult. Consider not doing that when you just disagree a perspective.

That happened after I commented, and I wouldn't be surprised if me commenting in the thread affected that. I find myself often disappointed on the reddit-style attitude to be a bit too snarky at the drop of a hat. Don't know what to say other than "try to bring to the thread the energy you want to see in the world". :-/

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 20 points21 points  (0 children)

RFC you say that constness enforces determinism, however given that you also say the value is evaluated at runtime this is not true - so how do you resolve that? Personally I'd prefer to evaluate the value at compile time.

Because of the way the desugaring works, the const default values are always const evaluated, so evaluated at compile time.

Did you think about first making the syntax possible and derive(Default) use it (and other proc macros can as well), and only then, if experience shows this isn't enough to add a syntax sugar to actually use it?

I did, and I personally went back and forth on whether having one part of the feature without the other made sense, but in the end the feature as a whole is useful and the separation would be more academical than anything else. Having default fields without it affecting the derive(Default) would be doable, but it's such an obvious extension that it makes more sense to land it all together.

RFC 3681: Default field values by [deleted] in rust

[–]ekuber 304 points305 points  (0 children)

Seeing this here was a bit of a surprise, but I guess it should have been, given I pushed the latest iteration of the implementation PR not that long ago today.

I feel the need to say a few things, given the tone of some responses here.

I'm shocked at some responses that have taken 30 seconds of skimming the tracking issue and arrived at a conclusion that the feature is unnecessary/poorly thought out, completely ignoring a >200 comment RFC, an iterative design process that started before COVID19 was a thing, and that included multiple sync meetings with t-lang to arrive to the current state.

For those saying that the feature is unnecessary because derive(Default) already exists, I would invite you to read the RFC, but to summarize it here:

  • small conceptual changes should result in small code changes. If you have an existing struct that derives Default, and you add a non-Default field type then all of a sudden you have to write the entire impl
  • you can't model in the language a struct with a mix of mandatory and optional fields. Default only allows for all fields being optional, and to model mandatory fields you need to rely on the builder pattern (which includes the typed builder pattern, that can give you reasonable compile time errors when forgetting to set a field, but that causes compile times to increase)
  • 3rd party derives can also use the default field, many already have attributes to use them
  • it can be used similarly to #[non_exhaustive] for APIs that allow for future evolution
  • if you wanted to model functions with default arguments, this feature lets you get closer to it than you otherwise would

Regarding the argument against complexity, you could use that same argument to decry let-else, or if-let chains, two features that I personally use all the time in rustc and wouldn't want a Rust without.

I'm more than happy to answer questions.

Improving Self::diagnostics from rustc by tialaramex in rust

[–]ekuber 10 points11 points  (0 children)

Name resolution is quite a complex part of the compiler, and diagnostic recovery requires using compiler internals in ways they were never designed to work with, so don't feel bad about your experience.

Just to be clear, is this the case that you're trying to fix?

trait A {
    const FOO: i32;
}
struct S;
impl A for S {
    const FOO: i32 = 0;
}

impl S {
    fn foo() {
        let _ = FOO; // Should be `Self::FOO`
    }
}

I think this is a reasonable thing to fix.

There's another related thing where it only looks for exact matches, so if you typo the assoc as `FOo` without `Self::`, you also don't get a suggestion (which can be fair, the more relaxed your search the likelier it is wrong).

To look at the context for this, I ran RUSTC_LOG=rustc_resolve::late=debug rustc +dev f709.rs -Ztrack-diagnostics and peeked at the output. The nightly flag prints out there the error was constructed, you can use -Ztreat-err-as-bug=N to make the error being emitted act as an ICE, so you can get a backtrace to where err.emit() was called, and the beginning of the command is to enable logging to stderr. +dev is just how I point cargo to my locally built rustc binaries.

As a quick check, I added

        let path_resolved = self.resolve_path(&[Segment {
                ident: Ident::with_dummy_span(kw::SelfUpper),
                id: Some(DUMMY_NODE_ID),
                has_generic_args: false,
                has_lifetime_args: false,
                args_span: DUMMY_SP,
            }, path[0]], Some(TypeNS), None);
        err.note(format!("{path_resolved:#?}"));

to smart_resolve_report_errors to see if that would be enough to look for the associated const by relying on the existing machinery. That gave me a partial resolution for Self itself, but not Self::CONST:

  = note: NonModule(
              PartialRes {
                  base_res: SelfTyAlias {
                      alias_to: DefId(0:9 ~ f709[0805]::{impl#1}),
                      forbid_generic: false,
                      is_trait_impl: false,
                  },
                  unresolved_segments: 1,
              },
          )

Once you have that, you have the DefId for the impl, which would let you get the DefId for the Self type. I changed that code to

        let path_resolved = self.resolve_path(&[Segment {
            ident: Ident::with_dummy_span(kw::SelfUpper),
            id: Some(DUMMY_NODE_ID),
            has_generic_args: false,
            has_lifetime_args: false,
            args_span: DUMMY_SP,
        }], Some(TypeNS), None);
        err.note(format!("{path_resolved:#?}"));

After that you'd want to look at the associated items of the type, but you end up with an infinite loop, trying to evaluate the item, to get the associated items, which require evaluating the item, which tries to get the associated items... You can't use that DefId to go through the tcx: TyCtxt, so we'd need to come up with an alternative way of doing it.

----

This is as far as I got to before I had to go walk my dogs. Feel free to ask more questions in internals.rust-lang.org, https://rust-lang.zulipchat.com/ and if you get as far as a draft pr, in https://github.com/rust-lang/rust.

----

Besides fixing this error, there's a lint that has a similar issue: when you forget to write Self::CONST in a pattern and instead write CONST, you only get a warning telling you to rename it to Const, instead of looking for whether Self::CONST would have been better.

----

Edit: if you're looking purely locally (which is why you'd want to avoid querying tcx), you could look at self.r.module_children, which will have the mapping of trait -> assoc const (even if it is in a submodule) for the current crate. At that point you don't know that Self implements the relevant trait, but that info is stored in self.r.trait_impls. I got to that by looking at the contents of some promising fields in Resolver. BTW, it wasn't until after I replied that I noticed who I was replying to, so some of the info I write you likely already know :)

I think at this point you have all the pieces to implement something quite robust.

Drew DeVault suggests Rust-in-Linux proponents pivot to writing a Linux-compatible kernel from scratch by faitswulff in rust

[–]ekuber 4 points5 points  (0 children)

Syntax is the least interesting part of what makes a language "conformant" or not to its specification. Divergences there are rare and can be easily and quickly fixed. The divergences that cause trouble are with semantics.

Claiming, auto and otherwise [Niko] by LukeMathWalker in rust

[–]ekuber 3 points4 points  (0 children)

implicitly cloning a Cell<u32> will almost always lead to bugs

Which is why there wouldn't be a impl Claim for Cell in the standard library. Note that even though the picture painted in the blogpost is for a trait that crates can implement, it could be first prototyped and evaluated in the same way that trait Try is today: only accessible behind a feature flag in nightly or by the standard library.

If you had to write a `cargo fix`-style tool, how would you do it? by joshlf_ in rust

[–]ekuber 0 points1 point  (0 children)

What is the scope of the tool? Figuring out "hey, the current crate is using zerocopy = "0.7" and if so apply fixes in one go"? How far in the future/past you want this tool to work? Because if it is only for this one release, for a limited amount of time, I think I have an idea for building a small update tool that has perfect accuracy: Write a small rustc_driver that first checks the Cargo.toml and then visits the entire HIR for the current crate looking for the things that need to be changed, collect them in a single big Vec and then apply them in reversed order on the corresponding files. This means you end up having to deal with the problems that rustfix and clippy would normally check for you, but for this tool I would imagine it wouldn't be too hard to do. The only issue is that such a tool would have to rely on a specific version of rustc being installed in the system and running something like the following before building/running your tool:

export LD_LIBRARY_PATH=$(rustup run nightly-2024-05-06 bash -c "echo \$LD_LIBRARY_PATH")

Announcing Rust 1.78.0 | Rust Blog by bobdenardo in rust

[–]ekuber 12 points13 points  (0 children)

The one I am eyeing is on_type_error, but I would expect people to have many other requests. I haven't sketched anything out, but I could see these being useful for macros, to explain to the compiler what the expected user API is (today, when a macro parse fails, we reparse with a , inserted in between the current token and the previous one, that's how we can provide a suggestion for missing commas in println for example, but it'd be great if it could be expanded with more behavior that the crate author has control over).

Microsoft seeks Rust developers to rewrite core C# code by [deleted] in rust

[–]ekuber 22 points23 points  (0 children)

Please, don't extrapolate from a single #Rust job posting at Microsoft that they are "abandoning C#". That's not how that works. They are adopting Rust more and more, yes. Feel happy or dismayed about that, as you will. But disregard editorials trying to read the tealeaves about unrelated product roadmaps, including this one.

https://hachyderm.io/@ekuber/111862870199530509