More error checking for member type lookup

[jtbandes]

Fixes a couple compiler_crashers and an IDE crasher.

[DougGregor]

Thanks!

[slavapestov]

@jtbandes Thanks for looking into this, but ideally you should investigate why the nested type is missing instead of hiding the issue.

[slavapestov]

@jtbandes In particular, setting a type to ErrorType does not guarantee a diagnostic gets produced. ErrorType leaking out in the type checked AST will just crash later.

[jtbandes]

@slavapestov Thanks for the feedback. What would be the course of action after figuring out why it's missing? If you look at the test cases that were crashing, there is some clearly invalid syntax that produces ErrorType (and usually the diagnostic is produced immediately when that happens), so it seemed to me that propagating the ErrorType was the right thing to do. Is there some way of handling errors like this without propagating?

[slavapestov]

@jtbandes Propagating an ErrorType like in the other commit is fine. In this commit, you're returning an ErrorType when getNestedType() returns null. I don't think the that's supposed to happen because we should have already resolved the name by then, so we should figure out the root cause, but I might be wrong.

Do you want to try reducing one of the crashing test cases further, then it might be obvious what's going on.

[jtbandes]

OK, I can take a look later this evening.

[slavapestov]

Looking at this test case,

protocol A1{
typealias e
class B{
protocol A2{
typealias f=e
#^A^#

It crashes trying to look up 'e'.

If I remove the 'class B{' in the middle, it stops crashing and correctly diagnoses errors.

The code is invalid anyway so there's not much point making this work too much, but its worth figuring out why it can't find 'e' when the class is in the middle. It might help you fix a more fundamental issue that will resolve more compiler crashers.

[jtbandes]

@slavapestov It looks like this code doesn't traverse more than one level of parents, so adding the class B{ in between produces an archetype with no parent.

I don't know what the right course of action is — I'm not sure why it would ever be okay for getMemberForBaseType to assume that archetypeParent->getParent() is non-null, so it still makes sense to me to leave in this null check. However I'm not sure if traversing up more than one level of parent type is necessary either. What do you think?

[slavapestov]

@jtbandes I think the problem is that when we look up 'e', we're looking it up in the archetype for the inner protocol's 'Self', when it should be using the outer 'Self'.

The reason this code assumes the archetype exists is that we should have already caught invalid type names by now. The name lookup code found the decl for the type 'e', but the archetype builder didn't create an archetype for it.

However this sounds like it might be a tedious fix, and nested protocols is not something we ever want to support. So unless you see an opportunity to improve some existing logic here, I think we can go with the targeted fix for now.