Skip to content

[verifier] [cxx-interop] Remove empty return result from constructors. #32656

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

Conversation

zoecarver
Copy link
Contributor

Set the synthesized constructor's return result to nullptr and add an ASTVerifier check that the constructor's return statement does not contain a result.

Refs #32534.

@zoecarver zoecarver added the c++ interop Feature: Interoperability with C++ label Jul 1, 2020
@zoecarver zoecarver requested a review from gribozavr July 1, 2020 17:27
Out << "Expected ReturnStmt not to have a result. A constructor should not return a result. Returned expression: ";
S->getResult()->dump(Out);
Out << "\n";
abort();
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to double-check, return nil in failable constructors does not trigger this new error, right? It should be covered by existing tests I think, but worth checking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A failable constructor will return an Optional<Class> where the element is .none. So, this error is not triggered (also, for good measure, I ran a test program and it didn't trigger this error).

Set the synthesized constructor's return result to nullptr and add an ASTVerifier check that the constructor's return statement does not contain a result.
@zoecarver zoecarver force-pushed the verifier/ast/check-result-return branch from 50449eb to 0605ef4 Compare July 7, 2020 17:43
@zoecarver
Copy link
Contributor Author

@swift-ci please smoke test and merge.

@swift-ci swift-ci merged commit 7d6abb8 into swiftlang:master Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ interop Feature: Interoperability with C++
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants