Skip to content

TypeCheckType: Fix some bugs in the any syntax checker #72659

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 5, 2024

Conversation

AnthonyLatsis
Copy link
Collaborator

@AnthonyLatsis AnthonyLatsis commented Mar 28, 2024

Resolves #65026, resolves #65027, resolves #72588.

Diff counts are mostly due to tests.

@AnthonyLatsis

This comment was marked as duplicate.

@AnthonyLatsis

This comment was marked as duplicate.

@AnthonyLatsis

This comment was marked as duplicate.

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test macOS

@AnthonyLatsis
Copy link
Collaborator Author

@slavapestov @hborla ping

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have incorporated the tests I deleted here into test/type/explicit_existential.swift and test/type/explicit_existential_swift6.swift

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test Linux

Comment on lines -106 to +108
// expected-note@-1 2 {{protocol 'CircleMiddle' declared here}}
protocol CircleStart : CircleEnd { func circle_start() } // expected-error 2 {{protocol 'CircleStart' refines itself}}
protocol CircleEnd : CircleMiddle { func circle_end()} // expected-note 2 {{protocol 'CircleEnd' declared here}}
// expected-note@-1 3 {{protocol 'CircleMiddle' declared here}}
protocol CircleStart : CircleEnd { func circle_start() } // expected-error 3 {{protocol 'CircleStart' refines itself}}
protocol CircleEnd : CircleMiddle { func circle_end()} // expected-note 3 {{protocol 'CircleEnd' declared here}}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is unfortunate although definitely not a ExistentialTypeSyntaxChecker problem.

@AnthonyLatsis

This comment was marked as duplicate.

1 similar comment
@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test Linux

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please test source compatibility release

@AnthonyLatsis
Copy link
Collaborator Author

@hborla ping

@AnthonyLatsis
Copy link
Collaborator Author

@slavapestov ping

@@ -6097,13 +6128,32 @@ class ExistentialTypeSyntaxChecker : public ASTWalker {
return;
}

// Backtrack the stack, looking just through parentheses and metatypes. If
// we find an inverse (which always requires `any`), diagnose it specially.
Copy link
Contributor

Choose a reason for hiding this comment

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

You might be able to just pass down the outermost location instead of "backtracking" here, but it's fine the way it is

Copy link
Collaborator Author

@AnthonyLatsis AnthonyLatsis Jun 4, 2024

Choose a reason for hiding this comment

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

Thanks for sharing your thoughts on this, Slava.

You might be able to just pass down the outermost location

Is this roughly what you meant (passing tildeLoc down such that if valid here, then the ~ necessarily pertains to the given identifier type)?

PreWalkAction walkToTypeReprPre(TypeRepr *T) override {
  // ...
  if (auto *inverse = isa<InverseTypeRepr>(T)) {
    this->tildeLoc = inverse->getTildeLoc();
  } else if (this->tildeLoc.isValid()) {
    if (!T->isParenType() &&
        !isa<MetatypeTypeRepr>(T) &&
        !isa<DeclRefTypeRepr>(T)) {
      this->tildeLoc = SourceLoc();
    }
  }
  // ...
  
  checkDeclRefTypeRepr(declRefTR);
  
  this->tildeLoc = SourceLoc();

  // ...
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I’m going to try to get this into the release as is before it’s too late, but I’m still very much willing to discuss this alternative.

@AnthonyLatsis
Copy link
Collaborator Author

@swift-ci please smoke test

@AnthonyLatsis AnthonyLatsis merged commit f02d300 into swiftlang:main Jun 5, 2024
3 checks passed
@AnthonyLatsis AnthonyLatsis deleted the many-any-bugs branch June 5, 2024 15:19
AnthonyLatsis added a commit to AnthonyLatsis/swift that referenced this pull request Feb 12, 2025
…ntil Swift 7

swiftlang#72659 turned out to have some
source compatibility fallout that we need to fix. Instead of introducing
yet another brittle compatibility hack, stop emitting errors about a
missing `any` altogether until a future language mode.

Besides resolving the compatibility issue, this will encourage
developers to adopt any sooner and grant us ample time to gracefully
address any remaining bugs before the source compatibility burden
resurfaces.

A subsequent commit adds a diagnostic group that will allow users to
escalate these warnings to errors with `-Werror ExistentialAny`.
AnthonyLatsis added a commit to AnthonyLatsis/swift that referenced this pull request Feb 12, 2025
…ntil Swift 7

swiftlang#72659 turned out to have some
source compatibility fallout that we need to fix. Instead of introducing
yet another brittle compatibility hack, stop emitting errors about a
missing `any` altogether until a future language mode.

Besides resolving the compatibility issue, this will encourage
developers to adopt any sooner and grant us ample time to gracefully
address any remaining bugs before the source compatibility burden
resurfaces.

A subsequent commit adds a diagnostic group that will allow users to
escalate these warnings to errors with `-Werror ExistentialAny`.
AnthonyLatsis added a commit to AnthonyLatsis/swift that referenced this pull request Feb 12, 2025
…ntil Swift 7

swiftlang#72659 turned out to have some
source compatibility fallout that we need to fix. Instead of introducing
yet another brittle compatibility hack, stop emitting errors about a
missing `any` altogether until a future language mode.

Besides resolving the compatibility issue, this will encourage
developers to adopt any sooner and grant us ample time to gracefully
address any remaining bugs before the source compatibility burden
resurfaces.

A subsequent commit adds a diagnostic group that will allow users to
escalate these warnings to errors with `-Werror ExistentialAny`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants