profile
viewpoint
Michael Goulet compiler-errors @aws telegraph hill > sf > ca > usa i make the @rust-lang compiler better. types team, compiler team, diagnostics working group, and style team ⚙️🦀🧮.

compiler-errors/ferrodb 5

A small toy database written in Rust

compiler-errors/camp 4

A miniature, Rust-like toy programming language. ⁂ "41. The whole point of Camp is to dethrone the serious. Camp is playful, anti-serious." - Susan Sontag

compiler-errors/adelaide 3

Reimplementation of the Cheshire language, but featuring a demand-driven parser and revamping the language grammar

compiler-errors/codespan-derive 2

Derive macro for ergonomically creating a Diagnostic from an error macro

compiler-errors/project-rc 2

Reference-counted type that lets you project to fields (unsafe, untested)

compiler-errors/cheshire-old 1

The standard reference compiler for the Cheshire Programming Language.

compiler-errors/a-mir-formality 0

a PLT redex model of MIR and its type system

compiler-errors/async-trait 0

Type erasure for async trait methods

startedTodePond/DreamBerd

started time in 44 minutes

PullRequestReviewEvent

Pull request review commentrust-lang/rust

diagnostics: do not suggest type name tweaks on type-inferred closure args

 pub(super) fn check_fn<'a, 'tcx>(     let inputs_fn = fn_sig.inputs().iter().copied();     for (idx, (param_ty, param)) in inputs_fn.chain(maybe_va_list).zip(body.params).enumerate() {         // Check the pattern.+        let binding_span = try { body.params.get(idx)?.span };

Also I'd expect this indexing to be off if the function has var-args.

notriddle

comment created time in an hour

PullRequestReviewEvent

Pull request review commentrust-lang/rust

diagnostics: do not suggest type name tweaks on type-inferred closure args

 pub(super) fn check_fn<'a, 'tcx>(     let inputs_fn = fn_sig.inputs().iter().copied();     for (idx, (param_ty, param)) in inputs_fn.chain(maybe_va_list).zip(body.params).enumerate() {         // Check the pattern.+        let binding_span = try { body.params.get(idx)?.span };

isn't this just param.span?

notriddle

comment created time in an hour

PullRequestReviewEvent

pull request commentrust-lang/rust

Cleanup some `EarlyBinder::skip_binder()` -> `EarlyBinder::subst_identity()`

@bors r+ rollup

kylematsuda

comment created time in an hour

PullRequestReviewEvent

pull request commentrust-lang/rust

diagnostics: do not suggest type name tweaks on closure args

That's one way of doing it. If you don't want to do that much work though, you can check if the ty_span is equal to the binding's span, perhaps like how dc2977e630c233866459399f0ff773505372803d does it...

notriddle

comment created time in an hour

pull request commentrust-lang/rust

Add separate feature gate for async fn track caller

Also while we're at it, that line you mentioned:

allow_gen_future: Some([sym::gen_future, sym::closure_track_caller][..].into()),

We could actually be even more conservative and change it so that we only add sym::closure_track_caller if tcx.features().async_fn_track_caller is active... 🤔

bryangarza

comment created time in 2 hours

Pull request review commentrust-lang/rust

Add separate feature gate for async fn track caller

 // edition:2021+// revisions: afn nofeat  #![feature(async_closure, stmt_expr_attributes)]+#![cfg_attr(afn, feature(async_fn_track_caller))]  fn main() {     let _ = #[track_caller] async || {         //~^ ERROR `#[track_caller]` on closures is currently unstable [E0658]     }; }++#[track_caller]+async fn foo() {+    let _ = #[track_caller] async || {+        //~^ ERROR `#[track_caller]` on closures is currently unstable [E0658]+    };+}++#[track_caller]+async fn foo2() {+    let _ = async || {+        let _ = #[track_caller] async || {

These examples are kinda loaded. I was expecting something that tests things one at a time:

  • async fn with a regular closure with track caller on it
  • regular fn, with an async block, and inside that async block a regular closure with track caller on it.
  • etc.

The problem with really overloaded tests like this is that they may be failing due to a confluence of tests. Better to separate all the variables and make sure things don't slip thru the cracks.

bryangarza

comment created time in 2 hours

PullRequestReviewEvent
PullRequestReviewEvent

Pull request review commentrust-lang/rust

diagnostics: do not suggest type name tweaks on closure args

 pub(super) fn check_fn<'a, 'tcx>(         // for simple cases like `fn foo(x: Trait)`,         // where we would error once on the parameter as a whole, and once on the binding `x`.         if param.pat.simple_ident().is_none() && !params_can_be_unsized {-            fcx.require_type_is_sized(param_ty, param.pat.span, traits::SizedArgumentType(ty_span));+            fcx.require_type_is_sized(+                param_ty,+                param.pat.span,+                traits::SizedArgumentType(ty_span.filter(|_| !tcx.is_closure(fn_def_id.into()))),

Can you make this into an if like below? Easier to read that way IMO

notriddle

comment created time in 2 hours

PullRequestReviewEvent
PullRequestReviewEvent

pull request commentrust-lang/rust

Fix suggestion for matching struct with `..` on both ends

@bors r+

jieyouxu

comment created time in 2 hours

PullRequestReviewEvent

pull request commentrust-lang/rust

Use `load`+`store` instead of `memcpy` for small integer arrays

@bors r+

scottmcm

comment created time in 2 hours

PullRequestReviewEvent
PullRequestReviewEvent

Pull request review commentrust-lang/rust

Use `load`+`store` instead of `memcpy` for small integer arrays

 pub fn memcpy_ty<'a, 'tcx, Bx: BuilderMethods<'a, 'tcx>>(         return;     } -    bx.memcpy(dst, dst_align, src, src_align, bx.cx().const_usize(size), flags);+    if flags == MemFlags::empty()+        && let Some(bty) = bx.cx().scalar_copy_backend_type(layout)+    {+        // I look forward to only supporting opaque pointers+        let pty = bx.type_ptr_to(bty);+        let src = bx.pointercast(src, pty);+        let dst = bx.pointercast(dst, pty);

Yeah ok, so we explicitly want to be lowering these moves to vector types specifically because they can optimize better than arrays.

scottmcm

comment created time in 2 hours

PullRequestReviewEvent
PullRequestReviewEvent

Pull request review commentrust-lang/rust

Add separate feature gate for async fn track caller

 async fn foo_closure() {  // Since compilation is expected to fail for this fn when using // `nofeat`, we test that separately in `async-block.rs`-#[cfg(feat)]+#[cfg(cls)]

Well, I guess it's separate.

bryangarza

comment created time in 2 hours

PullRequestReviewEvent

Pull request review commentrust-lang/rust

Add separate feature gate for async fn track caller

 async fn foo_closure() {  // Since compilation is expected to fail for this fn when using // `nofeat`, we test that separately in `async-block.rs`-#[cfg(feat)]+#[cfg(cls)]

Why is this using cls and not afn? This is an async block -- shouldn't it only work when async_fn_track_caller is enabled?

bryangarza

comment created time in 2 hours

PullRequestReviewEvent

Pull request review commentrust-lang/rust

Safe Transmute: Enable handling references, including recursive types

 mod rustc {                 let dst = Tree::from_ty(dst, context);                  match (src, dst) {-                    // Answer `Yes` here, because 'unknown layout' and type errors will already+                    // Answer `Ok(None)` here, because 'unknown layout' and type errors will already                     // be reported by rustc. No need to spam the user with more errors.-                    (Err(Err::TypeError(_)), _) => Err(Answer::Yes),-                    (_, Err(Err::TypeError(_))) => Err(Answer::Yes),-                    (Err(Err::Unknown), _) => Err(Answer::Yes),-                    (_, Err(Err::Unknown)) => Err(Answer::Yes),-                    (Err(Err::Unspecified), _) => Err(Answer::No(Reason::SrcIsUnspecified)),-                    (_, Err(Err::Unspecified)) => Err(Answer::No(Reason::DstIsUnspecified)),+                    (Err(Err::TypeError(_)), _)+                    | (_, Err(Err::TypeError(_)))+                    | (Err(Err::Unknown), _)+                    | (_, Err(Err::Unknown)) => Err(Ok(None)),

So to summarize, maybe let's get rid of map_layouts altogether?

bryangarza

comment created time in 2 hours

PullRequestReviewEvent
more