[Sema] Fix compiler error when extending a typealias of a partially specialized generic type

[xavgru12]

Resolves #68212.

[AnthonyLatsis]

@swift-ci please smoke test Linux

[saehejkang]

i was curious to see how this was solved and guess I was overcomplicating the issue 😭

[AnthonyLatsis]

This is far from solved, FWIW 🙂. I only triggered the tests to encourage the author to iterate on failures before any feedback arrives.

[xedin]

Is it possible to make this PR as a draft?

[xavgru12]

I was converting to draft but Anthony was faster

[AnthonyLatsis]

For posterity, the convert to draft button is at the bottom of the Reviewers section.

[xavgru12]

@AnthonyLatsis
I changed the code according review and added test cases. Let me know what you think about this.

[xavgru12]

@AnthonyLatsis reminder

[xavgru12]

You once said a ping every two weeks is fine. So here I am pinging you @AnthonyLatsis

[AnthonyLatsis]

Thank you for the reminders and the patience. Another somewhat random test case I think worth having:

struct S1<T> {
  struct Inner<U> {}
}

struct S2<T> {
  typealias A1<U> = S1<T>.Inner<U>
  typealias A2<U> = S1<T>.Inner<U> where T == Int
}

extension S2<Int>.A1 {
  func foo1() {
    let int: Int
    let _: T = int // OK
  }
}

extension S2.A2 {
  func foo2() {
    let int: Int
    let _: T = int // OK
  }
}

[AnthonyLatsis]

If we remove isInferredType, this can be

Suggested change

	if (!typealiasBoundGenericType->hasTypeParameter()) {
	isInferredType = true;
	} else {
	isInferredType = false;
	break;
	}
	if (typealiasBoundGenericType->hasTypeParameter()) {
	return false;
	}

[xavgru12]

I removed isInferredType. I dislike the new solution because I have to write return true at the end of the function and I would like to have return false as default.

[AnthonyLatsis]

I prefer returns over writing to a variable because they are generally more robust and help to streamline the flow. Do you find it counterintuitive to return true at the end here? You could invert the name of the function, but probably not worth it right now; we need to come up with a more descriptive name.

[AnthonyLatsis]

We need to perform this check regardless of whether the original number of generic params matched.

[xavgru12]

Confused here, added a comment above about this

[AnthonyLatsis]

The comment: #73169 (comment)

---

So the goal is to relax the fall-through type alias criteria a little bit by allowing a mismatch in the number of generic parameters at the greatest depth, but only if that difference is due to some of those generic parameters being bound to fully concrete types. As for other depths, the current rules still apply. For example, <A><B,C>... vs. <A, B><C>... or <A, B, C>... vs. <A, B>... must not meet the criteria regardless of the nature of .... I.e., we should still return early if the size check or the std::equal check fail for what remains after we shave off max depth elements.

Could you rebase (I have renamed isGeneric to hasGenericParamList) and see if the test in test/decl and test/type pass with the current change? I would expect some of them to fail. If not, we should get this case covered.

[AnthonyLatsis]

I like how this appears to be unsafe but is actually safe because we are trimming a non-owning view into the array. But this is probably better:

Suggested change

	for (const auto &type : nominalGenericParams) {
	if (type->getDepth() == maxDepth) {
	nominalGenericParams = nominalGenericParams.drop_back();
	}
	}
	while (!nominalGenericParams.empty() &&
	nominalGenericParams.back().getDepth() == maxDepth) {
	nominalGenericParams = nominalGenericParams.drop_back();
	}

Test case for why we need !nominalGenericParams.empty():

struct S1<T> {
  struct Inner<U> {}
}
struct S2 {
  typealias A<T> = S1<T>.Inner<Int>
}
extension S2.A {
  func test() {
    let int: Int
    let _: U = int // error
  }
}

[xavgru12]

I add the test case and changed it to while.
The test does not produce an error.

So the nested struct parameter becomes Int. Then it checks for U which is Int now. Why is it supposed to produce an error?

[AnthonyLatsis]

I think this passes now because you moved the std::equal check into the if. I don’t think there is anything inherently wrong with this case, but it belongs to a larger family of cases, and supporting it properly will require more work.

[xavgru12]

@AnthonyLatsis
I iterated over it and added some comments unresolving threads. Please review again and I hope you are able to keep the overview over this, so let me know if something is unclear.

[xavgru12]

@AnthonyLatsis reminder

[xavgru12]

@AnthonyLatsis
another reminder, not sure if it was 2 weeks ago as Github suggests

[xavgru12]

@AnthonyLatsis another reminder

[AnthonyLatsis]

@slavapestov Could you take a look at this too?

[xavgru12]

@AnthonyLatsis
It has been some time. I think this is close to a good solution

[xavgru12]

@AnthonyLatsis could you have a look?

[xavgru12]

@AnthonyLatsis @slavapestov
Could someone have a look at it, so I can iterate further on it if required?

[xavgru12]

@AnthonyLatsis

[xavgru12]

swift typecheckdecl isGeneric seems to have changed since last time I worked on it. Can be reviewed either way though