Skip to content

Commit

Permalink
Sema: Nuke NominalTypeDecl::markInvalidGenericSignature()
Browse files Browse the repository at this point in the history
This would just set the NominalTypeDecl's declared type to
ErrorType, which caused problems elsewhere.

Instead, generalize the logic used for AbstractFunctionDecl.
This correctly wires up the GenericTypeParamDecl's archetypes even
if the signature didn't validate, fixing crashes if the generic
parameters of the type are referenced.
  • Loading branch information
slavapestov committed Dec 14, 2015
1 parent dfbb580 commit c258f99
Show file tree
Hide file tree
Showing 713 changed files with 728 additions and 735 deletions.
3 changes: 0 additions & 3 deletions include/swift/AST/Decl.h
Expand Up @@ -2871,9 +2871,6 @@ class NominalTypeDecl : public TypeDecl, public DeclContext,
return GenericSig;
}

/// Mark generic type signature as invalid.
void markInvalidGenericSignature();

/// getDeclaredType - Retrieve the type declared by this entity.
Type getDeclaredType() const { return DeclaredTy; }

Expand Down
7 changes: 0 additions & 7 deletions lib/AST/Decl.cpp
Expand Up @@ -1922,13 +1922,6 @@ void NominalTypeDecl::setGenericSignature(GenericSignature *sig) {
GenericSig = sig;
}

void NominalTypeDecl::markInvalidGenericSignature() {
ASTContext &ctx = getASTContext();
overwriteType(ErrorType::get(ctx));
if (!getDeclaredType())
setDeclaredType(ErrorType::get(ctx));
}

void NominalTypeDecl::computeType() {
assert(!hasType() && "Nominal type declaration already has a type");

Expand Down
28 changes: 15 additions & 13 deletions lib/Sema/TypeCheckDecl.cpp
Expand Up @@ -795,22 +795,24 @@ void TypeChecker::revertGenericParamList(GenericParamList *genericParams) {
}
}

static void markInvalidGenericSignature(AbstractFunctionDecl *AFD,
static void markInvalidGenericSignature(ValueDecl *VD,
TypeChecker &TC) {
ArchetypeBuilder builder = TC.createArchetypeBuilder(AFD->getParentModule());
auto genericParams = AFD->getGenericParams();

// If there is a parent context, add the generic parameters and requirements
// from that context.
auto dc = AFD->getDeclContext();

if (dc->isTypeContext())
if (auto sig = dc->getGenericSignatureOfContext())
builder.addGenericSignature(sig, true);
GenericParamList *genericParams;
if (auto *AFD = dyn_cast<AbstractFunctionDecl>(VD))
genericParams = AFD->getGenericParams();
else
genericParams = cast<NominalTypeDecl>(VD)->getGenericParams();

// If there aren't any generic parameters at this level, we're done.
if (!genericParams)
if (genericParams == nullptr)
return;

DeclContext *DC = VD->getDeclContext();
ArchetypeBuilder builder = TC.createArchetypeBuilder(DC->getParentModule());

if (DC->isTypeContext())
if (auto sig = DC->getGenericSignatureOfContext())
builder.addGenericSignature(sig, true);

// Visit each of the generic parameters.
for (auto param : *genericParams)
Expand Down Expand Up @@ -5824,7 +5826,7 @@ void TypeChecker::validateDecl(ValueDecl *D, bool resolveTypeParams) {

// Validate the generic type parameters.
if (validateGenericTypeSignature(nominal)) {
nominal->markInvalidGenericSignature();
markInvalidGenericSignature(nominal, *this);
return;
}

Expand Down
4 changes: 2 additions & 2 deletions test/Generics/associated_type_typo.swift
Expand Up @@ -49,11 +49,11 @@ func typoAssoc4<T : P2 where T.Assocp2.assoc : P3>(_: T) { }
// <rdar://problem/19620340>

func typoFunc1<T : P1>(x: TypoType) { // expected-error{{use of undeclared type 'TypoType'}}
let _: T.Assoc -> () = { let _ = $0 }
let _: T.Assoc -> () = { let _ = $0 } // expected-error{{'Assoc' is not a member type of 'T'}}
}

func typoFunc2<T : P1>(x: TypoType, y: T) { // expected-error{{use of undeclared type 'TypoType'}}
let _: T.Assoc -> () = { let _ = $0 }
let _: T.Assoc -> () = { let _ = $0 } // expected-error{{'Assoc' is not a member type of 'T'}}
}

func typoFunc3<T : P1>(x: TypoType, y: T.Assoc -> ()) { // expected-error{{use of undeclared type 'TypoType'}}
Expand Down
1 change: 1 addition & 0 deletions test/Sema/accessibility.swift
Expand Up @@ -309,6 +309,7 @@ struct DefaultGeneric<T> {}
struct DefaultGenericPrivate<T: PrivateProto> {} // expected-error {{generic struct must be declared private because its generic parameter uses a private type}}
struct DefaultGenericPrivate2<T: PrivateClass> {} // expected-error {{generic struct must be declared private because its generic parameter uses a private type}}
struct DefaultGenericPrivateReq<T where T == PrivateClass> {} // expected-error {{same-type requirement makes generic parameter 'T' non-generic}}
// expected-error@-1 {{generic struct must be declared private because its generic requirement uses a private type}}
struct DefaultGenericPrivateReq2<T where T: PrivateProto> {} // expected-error {{generic struct must be declared private because its generic requirement uses a private type}}

public struct PublicGenericInternal<T: InternalProto> {} // expected-error {{generic struct cannot be declared public because its generic parameter uses an internal type}}
Expand Down
2 changes: 1 addition & 1 deletion test/decl/protocol/req/recursion.swift
Expand Up @@ -14,7 +14,7 @@ public struct S<A: P where A.T == S<A>> {}

// rdar://problem/19840527
class X<T where T == X> { // expected-error{{same-type requirement makes generic parameter 'T' non-generic}}
var type: T { return self.dynamicType } // expected-error{{use of undeclared type 'T'}}
var type: T { return self.dynamicType } // expected-error{{cannot convert return expression of type 'X<T>.Type' to return type 'T'}}
}

protocol Y {
Expand Down
@@ -1,7 +1,7 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing

enum S{init(){enum S<T where S:T>:T
enum S{init(){enum S<T where S:T>:T
@@ -1,7 +1,7 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing

enum S<T where S:a>:T.j
enum S<T where S:a>:T.j
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse

// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down
@@ -1,4 +1,4 @@
// RUN: not --crash %target-swift-frontend %s -parse
// RUN: not %target-swift-frontend %s -parse
// Distributed under the terms of the MIT license
// Test case submitted to project by https://github.com/practicalswift (practicalswift)
// Test case found by fuzzing
Expand Down

6 comments on commit c258f99

@cwillmor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome!

@cwillmor
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To quantify just how awesome this is: Before this commit there were 783 compiler crashers that were failing. Now there are 74. So this commit fixed 91% of the outstanding compiler crashers.

@jtbandes
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎊 Nicely done!

@lattner
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Totally awesome Slava!

@practicalswift
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow! Thank you! ッ

@rae
Copy link

@rae rae commented on c258f99 Jan 14, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wish I could ⭐️ a commit

Please sign in to comment.