Skip to content

Conversation

@bitjammer
Copy link
Contributor

@bitjammer bitjammer commented Dec 6, 2016

The generic environment of a GenericTypeToArchetypeResolver may be
nullptr, as returned by DeclContext::getGenericEnvironmentOfContext,
during prechecking of closure expressions at the necessarily non-generic
top level.

Fixes a couple of crashers.

rdar://problem/28822213

The generic environment of a GenericTypeToArchetypeResolver may be
`nullptr`, as returned by DeclContext::getGenericEnvironmentOfContext,
during prechecking of closure expressions at the necessarily non-generic
top level.

Fixes a couple of crashers.
@bitjammer
Copy link
Contributor Author

@swift-ci Smoke test and merge

@slavapestov
Copy link
Contributor

We should instead figure out why he environment is null.

@swift-ci swift-ci merged commit e0bec70 into swiftlang:master Dec 6, 2016
@DougGregor
Copy link
Member

I have this same hack in my tree thats working to remove the partial generic-to-archetype resolver, which has a similar defensive check already.

@bitjammer
Copy link
Contributor Author

TypeCheckConstraints.cpp:940 says:

  GenericTypeToArchetypeResolver resolver(
      closure->getGenericEnvironmentOfContext());

DeclContext::getGenericEnvironmentOfContext can return nullptr if the closure's context is a module, file, or top level code. Maybe the hack was returning nullptr in the first place, don't you think? I don't think it's a bad thing to add a nullptr check if that's what we're going to use to represent an empty environment ...

@akyrtzi
Copy link
Contributor

akyrtzi commented Dec 6, 2016

I suggest reverting this so we can remember to fix the name lookup issues later.

I don't think this is a good idea, leave an easily reproducible crasher in the compiler for who knows how long (e.g. take a look at how many test cases remain unfixed in validation-test/compiler_crashers). Also see Doug's 6c7277b which is in the defensive spirit.

I think a better approach is to keep defensive checks that prevent crashers and open a JIRA bug to keep track investigating why something is not quite as we expected it to. If this investigation leads to not needing the defensive check then we could remove it as part of that JIRA fix.

For this I opened https://bugs.swift.org/browse/SR-3348

@slavapestov
Copy link
Contributor

@bitjammer A closure at module scope does not have a generic environment. The problem is that name lookup should not be finding the generic parameter in the first place in this case.

@bitjammer bitjammer deleted the guard-null-generic-env branch December 10, 2016 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants