Skip to content

[lit] Change all %target-swift-frontend invocations to verify ownership. #23260

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

Conversation

gottesmm
Copy link
Contributor

We are already doing this for most of the target-swift-frontend ones. In a
subsequent commit, I am going to remove the redundant ones.

NOTE: On Darwin, I have not enabled it on the %target-swift-frontend mock SDK
commands. I ran into an issue with one of the PrintAsObjC tests that I am still
tracking down. I would rather just get this turned on to prevent further
regressions.

I also updated a few tests that needed some small tweaks to pass
this. Specifically:

  1. Some parser tests needed some extra ossa insts to pass the verifier. This
    doesn't effect what they actually test.

  2. IRGen tests that should never have processed ossa directly. Today, we are
    working towards a world where IRGen never processes [ossa] directly. Instead we
    lower first. If/when that changes, we should add back in specific [ossa] tests.

  3. A singular SILOptimizer definite init test case where the ownership verifier
    fails due to a case which DI already flags as illegal (we just crash earlier). I
    am going to look into fixing that by putting in errors in the typechecker or in
    SILGen (not sure yet). I changed it to use target-swiftc_driver which does not
    have ownership verification enabled.

We are already doing this for most of the target-swift-frontend ones. In a
subsequent commit, I am going to remove the redundant ones.

NOTE: On Darwin, I have not enabled it on the %target-swift-frontend mock SDK
commands. I ran into an issue with one of the PrintAsObjC tests that I am still
tracking down. I would rather just get this turned on to prevent further
regressions.

I also updated a few tests that needed some small tweaks to pass
this. Specifically:

1. Some parser tests needed some extra ossa insts to pass the verifier. This
doesn't effect what they actually test.

2. IRGen tests that should never have processed ossa directly. Today, we are
working towards a world where IRGen never processes [ossa] directly. Instead we
lower first. If/when that changes, we should add back in specific [ossa] tests.

3. A singular SILOptimizer definite init test case where the ownership verifier
fails due to a case which DI already flags as illegal (we just crash earlier). I
am going to look into fixing that by putting in errors in the typechecker or in
SILGen (not sure yet). I changed it to use target-swiftc_driver which does not
have ownership verification enabled.
@gottesmm
Copy link
Contributor Author

@swift-ci smoke test and merge

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test OSX platform

@gottesmm
Copy link
Contributor Author

@swift-ci smoke test OS X platform

@gottesmm gottesmm merged commit b84928f into swiftlang:master Mar 13, 2019
@gottesmm gottesmm deleted the pr-285a2392739649b4abf8d4b3a3464c983416b4e0 branch March 13, 2019 06:37
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.

1 participant