diagnostics: avoid mismatch between variance index and hir generic #116045

notriddle · 2023-09-22T00:24:32Z

This happens because variances are constructed from ty generics, and ty generics are always constructed with lifetimes first.

rust/compiler/rustc_hir_analysis/src/collect/generics_of.rs

Lines 248 to 269 in b3aa8e7

    
               params.extend(early_lifetimes.enumerate().map(|(i, param)| ty::GenericParamDef { 
        
                   name: param.name.ident().name, 
        
                   index: own_start + i as u32, 
        
                   def_id: param.def_id.to_def_id(), 
        
                   pure_wrt_drop: param.pure_wrt_drop, 
        
                   kind: ty::GenericParamDefKind::Lifetime, 
        
               })); 
        
               // Now create the real type and const parameters. 
        
               let type_start = own_start - has_self as u32 + params.len() as u32; 
        
               let mut i: u32 = 0; 
        
               let mut next_index = || { 
        
                   let prev = i; 
        
                   i += 1; 
        
                   prev + type_start 
        
               }; 
        
               const TYPE_DEFAULT_NOT_ALLOWED: &'static str = "defaults for type parameters are only allowed in \ 
        
               `struct`, `enum`, `type`, or `trait` definitions"; 
        
               params.extend(ast_generics.params.iter().filter_map(|param| match param.kind { 
        
                   GenericParamKind::Lifetime { .. } => None,

Fixes #83556

This happens because variances are constructed from ty generics, and ty generics are always constructed with lifetimes first. See compiler/rustc_hir_analysis/src/collect/generics_of.rs:248-269 Fixes rust-lang#83556

rustbot · 2023-09-22T00:24:39Z

r? @cjgillot

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

cjgillot · 2023-09-22T11:15:22Z

compiler/rustc_hir_analysis/src/check/wfcheck.rs

+            for hp in hir_generics.params {
+                if hp.name.ident().name == ty_param.name {
+                    hir_param = hp;
+                    break;
+                }
+            }


Should we just silence the error? The program is invalid anyway, and we check that by delaying a bug.

Suggested change

for hp in hir_generics.params {

if hp.name.ident().name == ty_param.name {

hir_param = hp;

break;

}

}

continue;

Yeah, that seems reasonable.

cjgillot · 2023-09-22T11:17:15Z

compiler/rustc_hir_analysis/src/check/wfcheck.rs

+        let ty_param = &ty_generics.params[index];
+        let mut hir_param = &hir_generics.params[index];
+
+        if ty_param.name != hir_param.name.ident().name {


Suggested change

if ty_param.name != hir_param.name.ident().name {

if ty_param.def_id != hir_param.def_id {

Working with def_ids is always more robust.

Good catch.

cjgillot · 2023-09-22T11:19:34Z

compiler/rustc_hir_analysis/src/check/wfcheck.rs

+            for hp in hir_generics.params {
+                if hp.name.ident().name == ty_param.name {
+                    hir_param = hp;
+                    break;
+                }
+            }


Suggested change

for hp in hir_generics.params {

if hp.name.ident().name == ty_param.name {

hir_param = hp;

break;

}

}

if let Some(node) = tcx.find_node_by_def_id(ty_param.def_id) {

hir_param = node.expect_generic_param();

}

cjgillot · 2023-09-23T07:10:06Z

Thanks.
@bors r+

bors · 2023-09-23T07:10:08Z

📌 Commit 58ef3a0 has been approved by cjgillot

It is now in the queue for this repository.

bors · 2023-09-23T08:12:03Z

⌛ Testing commit 58ef3a0 with merge 0237aa3...

bors · 2023-09-23T10:01:43Z

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 0237aa3 to master...

rust-timer · 2023-09-23T11:23:36Z

Finished benchmarking commit (0237aa3): comparison URL.

Overall result: no relevant changes - no action needed

@rustbot label: -perf-regression

Instruction count

This benchmark run did not return any relevant results for this metric.

Max RSS (memory usage)

Results

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	4.6%	[4.6%, 4.6%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 633.587s -> 634.28s (0.11%)
Artifact size: 317.57 MiB -> 317.90 MiB (0.10%)

diagnostics: avoid mismatch between variance index and hir generic

3799af3

This happens because variances are constructed from ty generics, and ty generics are always constructed with lifetimes first. See compiler/rustc_hir_analysis/src/collect/generics_of.rs:248-269 Fixes rust-lang#83556

rustbot assigned cjgillot Sep 22, 2023

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Sep 22, 2023

cjgillot reviewed Sep 22, 2023

View reviewed changes

diagnostics: simpler 83556 handling by bailing out

58ef3a0

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 23, 2023

bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 23, 2023

bors merged commit 0237aa3 into rust-lang:master Sep 23, 2023

rustbot added this to the 1.74.0 milestone Sep 23, 2023

notriddle deleted the notriddle/issue-83556 branch September 26, 2023 23:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

diagnostics: avoid mismatch between variance index and hir generic #116045

diagnostics: avoid mismatch between variance index and hir generic #116045

notriddle commented Sep 22, 2023

rustbot commented Sep 22, 2023

cjgillot Sep 22, 2023

notriddle Sep 23, 2023

cjgillot Sep 22, 2023

notriddle Sep 23, 2023

cjgillot Sep 22, 2023

cjgillot commented Sep 23, 2023

bors commented Sep 23, 2023

bors commented Sep 23, 2023

bors commented Sep 23, 2023

rust-timer commented Sep 23, 2023

	params.extend(early_lifetimes.enumerate().map(\|(i, param)\| ty::GenericParamDef {
	name: param.name.ident().name,
	index: own_start + i as u32,
	def_id: param.def_id.to_def_id(),
	pure_wrt_drop: param.pure_wrt_drop,
	kind: ty::GenericParamDefKind::Lifetime,
	}));

	// Now create the real type and const parameters.
	let type_start = own_start - has_self as u32 + params.len() as u32;
	let mut i: u32 = 0;
	let mut next_index = \|\| {
	let prev = i;
	i += 1;
	prev + type_start
	};

	const TYPE_DEFAULT_NOT_ALLOWED: &'static str = "defaults for type parameters are only allowed in \
	`struct`, `enum`, `type`, or `trait` definitions";

	params.extend(ast_generics.params.iter().filter_map(\|param\| match param.kind {
	GenericParamKind::Lifetime { .. } => None,

	if ty_param.name != hir_param.name.ident().name {
	if ty_param.def_id != hir_param.def_id {

diagnostics: avoid mismatch between variance index and hir generic #116045

diagnostics: avoid mismatch between variance index and hir generic #116045

Conversation

notriddle commented Sep 22, 2023

rustbot commented Sep 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cjgillot commented Sep 23, 2023

bors commented Sep 23, 2023

bors commented Sep 23, 2023

bors commented Sep 23, 2023

rust-timer commented Sep 23, 2023

Overall result: no relevant changes - no action needed