regression: new resolution failures in 1.74 #117056

Mark-Simulacrum · 2023-10-22T15:10:06Z

I'm grouping these together but we should make sure all of them are fixed / tested before closing this:

https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/gh/CaviesLabs.hamsterswap-program/log.txt
- Bisected to resolve: mark binding is determined after all macros had been expanded #115269, but didn't appear as a regression in its crater run
https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/gh/RatioFinance.ratio-staking/log.txt
- Bisected to resolve: mark binding is determined after all macros had been expanded #115269, but didn't appear as a regression in its crater run
https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/gh/nosana-ci.nosana-jobs/log.txt
- Bisected to resolve: mark binding is determined after all macros had been expanded #115269, but didn't appear as a regression in its crater run
https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/reg/nmap-analyze-1.0.0/log.txt
- Known regression resolve: mark binding is determined after all macros had been expanded #115269. Deemed acceptable due to bug fix.
https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/reg/luminance-gl-0.19.1/log.txt
- Bisected to Properly consider binder vars in HasTypeFlagsVisitor #115834
https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/reg/luminance-webgl-0.6.1/log.txt
- Bisected to Properly consider binder vars in HasTypeFlagsVisitor #115834

cc #117053 which is a similar-looking problem, maybe duplicate of this

[INFO] [stdout] error[E0425]: cannot find function `create_lookup_table` in module `instruction`
[INFO] [stdout]   --> programs/swap/src/action/create_address_lookup_table.rs:51:26
[INFO] [stdout]    |
[INFO] [stdout] 51 |             instruction::create_lookup_table(
[INFO] [stdout]    |                          ^^^^^^^^^^^^^^^^^^^ not found in `instruction`
[INFO] [stdout]    |
[INFO] [stdout] help: consider importing this function
[INFO] [stdout]    |
[INFO] [stdout] 1  + use solana_address_lookup_table_program::instruction::create_lookup_table;
[INFO] [stdout]    |
[INFO] [stdout] help: if you import `create_lookup_table`, refer to it directly
[INFO] [stdout]    |
[INFO] [stdout] 51 -             instruction::create_lookup_table(
[INFO] [stdout] 51 +             create_lookup_table(

The text was updated successfully, but these errors were encountered:

apiraino · 2023-10-23T10:56:48Z

I bisected to something different (if my bisection is correct): rollup eb76764 and specifically it says "Regression in rust-lang-ci@7e4f5ec" so cc @jhpratt ?

searched nightlies: from nightly-2023-04-30 to nightly-2023-10-23
regressed nightly: nightly-2023-06-29
searched commit range: 5ea6668...5bd28f5
regressed commit: eb76764

bisected with cargo-bisect-rustc v0.6.7

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc 2023-04-30 --regress error -- check

apiraino · 2023-10-23T10:58:08Z

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-high

lqd · 2023-10-23T11:11:50Z

@apiraino That rollup landed in 1.72.

lqd · 2023-10-23T11:45:06Z

As mark suggests there are different sources here. I haven't checked all of them, but the last 3 look like this:

luminance-gl-0.19.1 bisects to nightly-2023-09-15, rollup Rollup of 4 pull requests #115848, PR Properly consider binder vars in HasTypeFlagsVisitor #115834 cc author @compiler-errors.
luminance-webgl-0.6.1 is slightly more involved to bisect because it uses target wasm32-unknown-unknown that we don't build on unrolled perf builds, but points towards the same nightly/rollup. The error looks the same as the -gl version so it's probably safe to assume it would point at the same PR Properly consider binder vars in HasTypeFlagsVisitor #115834.
nmap-analyze-1.0.0 points to resolve: mark binding is determined after all macros had been expanded #115269, but there was a crater run for it: this crate did indeed show up in the results. As a bug fix, the breakage was deemed acceptable and PRs were opened to fix the 3 broken crates. Here's the one fixing nmap-analyze.

jhpratt · 2023-10-23T23:15:00Z

I haven't had anything land in this release cycle, so a bisection that leads to me must be incorrect.

compiler-errors · 2023-10-24T02:20:53Z

I'll investigate luminance-{,web}gl tomorrow.

apiraino · 2023-10-24T13:39:00Z

thanks @lqd (and sorry for the wrong bisection and ping)

jhpratt · 2023-10-24T14:51:17Z

No worries!

wesleywiser · 2023-10-26T16:14:38Z

Bisected nosana-ci.nosana-jobs:

searched toolchains b053db2 through b11431e

Regression in 735bb7e

apiraino · 2023-10-26T19:19:47Z

thanks @wesleywiser. Also tagging @bvanjoi about this beta regression since author of #115269

lqd · 2023-10-26T19:41:09Z

Also cc reviewer @petrochenkov, if you two could also weigh in on the first 3 regressions, which didn't appear in the crater run for #115269:

Felix has been doing some digging on the regressions themselves in this zulip thread.

bvanjoi · 2023-10-27T04:03:02Z

reduced for hamesterswap:

mod b {
  pub mod instruction {
    pub fn b() {}
  }
}

use b::*;

macro_rules! m {
  () => {
    pub mod instruction {
      pub fn a() {}
    }        
  };
}


// the `hamesterswap-program` uses a procedural macro to generate the instruction module.
// I believe its effect is similar to that of declarative macros
m!();


mod mm {
  use crate::*;
  fn f() {
    instruction::b();
  }
}

I think this is the expected behavior.

bvanjoi · 2023-10-27T05:13:56Z

reduced for ratio-staking:

// rustc code.rs --cfg 'feature="cpi"'
mod common {
  pub mod cpi {
    pub fn a() {} 
  }

  pub mod macros {
    #[macro_export]
    macro_rules! g {
        () => {
          cpi::a()
        }
    }
  }
}

use common::*;

macro_rules! m {
  () => {
    #[cfg(feature = "cpi")]
    pub mod cpi {
      pub fn b() {}
    }
  };
}

m!();

mod instructions {
  mod stake {
    use crate::*;    
    fn f() {
      g!()
    }
  }
}

I also think that this is the anticipated behavior

Moreover, nosana-jobs employs the same logic as this.

…try> Make sure that predicates with unmentioned bound vars are still considered global in the old solver In the old solver, we consider predicates with late-bound vars to not be "global": https://github.com/rust-lang/rust/blob/9c8a2694fadf3900c4d7880f6357cee60e9aa39b/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1840-L1844 The implementation of `has_late_bound_vars` was modified in rust-lang#115834 so that we'd properly anonymize binders that had late-bound vars but didn't reference them. This fixed an ICE. However, this also led to a behavioral change in rust-lang#117056 (comment) for a couple of crates, which now consider `for<'a> GL33: Shader` (note the binder var that is *not* used in the predicate) to not be "global". This forces associated types to not be normalizable due to the old trait solver being dumb. This PR distinguishes types which *reference* late-bound vars and binders which *have* late-bound vars. The latter is represented with the new type flag `TypeFlags::HAS_BINDER_VARS`, which is used when we only care about knowing whether binders have vars in their bound var list (even if they're not used, like for binder anonymization). This should fix (after beta backport) the `luminance-gl` and `luminance-webgl` crates in rust-lang#117056. r? types **(priority is kinda high on a review here given beta becomes stable on November 16.)**

…ackh726 Make sure that predicates with unmentioned bound vars are still considered global in the old solver In the old solver, we consider predicates with late-bound vars to not be "global": https://github.com/rust-lang/rust/blob/9c8a2694fadf3900c4d7880f6357cee60e9aa39b/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1840-L1844 The implementation of `has_late_bound_vars` was modified in rust-lang#115834 so that we'd properly anonymize binders that had late-bound vars but didn't reference them. This fixed an ICE. However, this also led to a behavioral change in rust-lang#117056 (comment) for a couple of crates, which now consider `for<'a> GL33: Shader` (note the binder var that is *not* used in the predicate) to not be "global". This forces associated types to not be normalizable due to the old trait solver being dumb. This PR distinguishes types which *reference* late-bound vars and binders which *have* late-bound vars. The latter is represented with the new type flag `TypeFlags::HAS_BINDER_VARS`, which is used when we only care about knowing whether binders have vars in their bound var list (even if they're not used, like for binder anonymization). This should fix (after beta backport) the `luminance-gl` and `luminance-webgl` crates in rust-lang#117056. r? types **(priority is kinda high on a review here given beta becomes stable on November 16.)**

Make sure that predicates with unmentioned bound vars are still considered global in the old solver In the old solver, we consider predicates with late-bound vars to not be "global": https://github.com/rust-lang/rust/blob/9c8a2694fadf3900c4d7880f6357cee60e9aa39b/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1840-L1844 The implementation of `has_late_bound_vars` was modified in #115834 so that we'd properly anonymize binders that had late-bound vars but didn't reference them. This fixed an ICE. However, this also led to a behavioral change in rust-lang/rust#117056 (comment) for a couple of crates, which now consider `for<'a> GL33: Shader` (note the binder var that is *not* used in the predicate) to not be "global". This forces associated types to not be normalizable due to the old trait solver being dumb. This PR distinguishes types which *reference* late-bound vars and binders which *have* late-bound vars. The latter is represented with the new type flag `TypeFlags::HAS_BINDER_VARS`, which is used when we only care about knowing whether binders have vars in their bound var list (even if they're not used, like for binder anonymization). This should fix (after beta backport) the `luminance-gl` and `luminance-webgl` crates in #117056. r? types **(priority is kinda high on a review here given beta becomes stable on November 16.)**

apiraino · 2023-12-14T10:59:47Z

A few fixes landed since a few weeks. Can this issue be closed?

wesleywiser · 2023-12-28T16:49:59Z

Revisting, of the 6 crater failures found above, two have been fixed leaving the following:

https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/gh/CaviesLabs.hamsterswap-program/log.txt
- Bisected to resolve: mark binding is determined after all macros had been expanded #115269, but didn't appear as a regression in its crater run
https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/gh/RatioFinance.ratio-staking/log.txt
- Bisected to resolve: mark binding is determined after all macros had been expanded #115269, but didn't appear as a regression in its crater run
https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/gh/nosana-ci.nosana-jobs/log.txt
- Bisected to resolve: mark binding is determined after all macros had been expanded #115269, but didn't appear as a regression in its crater run
https://crater-reports.s3.amazonaws.com/beta-1.74-4/beta-2023-10-21/reg/nmap-analyze-1.0.0/log.txt
- Known regression resolve: mark binding is determined after all macros had been expanded #115269. Deemed acceptable due to bug fix.

The first three are all instances of the same underlying issue and the fourth was known to be a bug fix. It sounds like this is anticipated fallout from the change to fix issues in macro expansion and a PR has been submitted the active project in that set. I'm therefore going to close this issue.

Make sure that predicates with unmentioned bound vars are still considered global in the old solver In the old solver, we consider predicates with late-bound vars to not be "global": https://github.com/rust-lang/rust/blob/9c8a2694fadf3900c4d7880f6357cee60e9aa39b/compiler/rustc_trait_selection/src/traits/select/mod.rs#L1840-L1844 The implementation of `has_late_bound_vars` was modified in #115834 so that we'd properly anonymize binders that had late-bound vars but didn't reference them. This fixed an ICE. However, this also led to a behavioral change in rust-lang/rust#117056 (comment) for a couple of crates, which now consider `for<'a> GL33: Shader` (note the binder var that is *not* used in the predicate) to not be "global". This forces associated types to not be normalizable due to the old trait solver being dumb. This PR distinguishes types which *reference* late-bound vars and binders which *have* late-bound vars. The latter is represented with the new type flag `TypeFlags::HAS_BINDER_VARS`, which is used when we only care about knowing whether binders have vars in their bound var list (even if they're not used, like for binder anonymization). This should fix (after beta backport) the `luminance-gl` and `luminance-webgl` crates in #117056. r? types **(priority is kinda high on a review here given beta becomes stable on November 16.)**

Mark-Simulacrum added this to the 1.74.0 milestone Oct 22, 2023

rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Oct 22, 2023

Mark-Simulacrum mentioned this issue Oct 22, 2023

1.74.0 crater runs #116567

Closed

apiraino removed the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Oct 23, 2023

rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 23, 2023

bvanjoi mentioned this issue Oct 27, 2023

fix a glob resolution issue nosana-ci/nosana-programs#4

Open

saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Oct 28, 2023

compiler-errors mentioned this issue Nov 4, 2023

Make sure that predicates with unmentioned bound vars are still considered global in the old solver #117589

Merged

wesleywiser added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. and removed regression-from-stable-to-beta Performance or correctness regression from stable to beta. labels Dec 28, 2023

wesleywiser closed this as completed Dec 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

regression: new resolution failures in 1.74 #117056

regression: new resolution failures in 1.74 #117056

Mark-Simulacrum commented Oct 22, 2023 •

edited by lqd

Loading

apiraino commented Oct 23, 2023

apiraino commented Oct 23, 2023

lqd commented Oct 23, 2023 •

edited

Loading

lqd commented Oct 23, 2023

jhpratt commented Oct 23, 2023

compiler-errors commented Oct 24, 2023

apiraino commented Oct 24, 2023

jhpratt commented Oct 24, 2023

wesleywiser commented Oct 26, 2023

apiraino commented Oct 26, 2023

lqd commented Oct 26, 2023

bvanjoi commented Oct 27, 2023

bvanjoi commented Oct 27, 2023 •

edited

Loading

apiraino commented Dec 14, 2023

wesleywiser commented Dec 28, 2023

regression: new resolution failures in 1.74 #117056

regression: new resolution failures in 1.74 #117056

Comments

Mark-Simulacrum commented Oct 22, 2023 • edited by lqd Loading

apiraino commented Oct 23, 2023

apiraino commented Oct 23, 2023

lqd commented Oct 23, 2023 • edited Loading

lqd commented Oct 23, 2023

jhpratt commented Oct 23, 2023

compiler-errors commented Oct 24, 2023

apiraino commented Oct 24, 2023

jhpratt commented Oct 24, 2023

wesleywiser commented Oct 26, 2023

apiraino commented Oct 26, 2023

lqd commented Oct 26, 2023

bvanjoi commented Oct 27, 2023

bvanjoi commented Oct 27, 2023 • edited Loading

apiraino commented Dec 14, 2023

wesleywiser commented Dec 28, 2023

Mark-Simulacrum commented Oct 22, 2023 •

edited by lqd

Loading

lqd commented Oct 23, 2023 •

edited

Loading

bvanjoi commented Oct 27, 2023 •

edited

Loading