Skip to content

[ownership-verifier] SubObject Borrowing + Better output for FileCheck + More tests #6972

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 3 commits into from
Jan 22, 2017

Conversation

gottesmm
Copy link
Contributor

This PR contains 3 different commits:

  1. f462d20. This commit renames PrintMessageInsteadOfAssert => IsSILOwnershipTestingEnabled and disables the partial ownership verification that occurs via SILInstruction::verifyOperandOwnership(). This is invoked by SILBuilder to catch invalid ownership early when an instruction is created, rather than later during full verification. This is bad from the perspective of FileCheck testing since use errors will come up twice. Since PrintMessageInsteadOfAssert has a bigger purpose now, it makes sense to rename it.

  2. 60449d5. This commit just adds 2 tests that shows that the verifier allows through multiple levels of consumed aggregates being constructed.

  3. f857f2c. This commit implements and adds tests for subobject borrowing.

rdar://29791263

…shipVerifierTestingEnabled and use it to disable SILInstruction::verifyOperandOwnership

The method SILInstruction::verifyOperandOwnership() is used in SILBuilder to
ensure that as instructions are created, if there are any use failures, the
error is caught at the moment the instruction is created. This makes debugging
such failures trivial. *NOTE* This does not cause dataflow verification to
occur.

The problem is that when PrintMessageInsteadOfAssert is enabled, this causes
dataflow failures to have their error messages emitted twice, complicating
FileCheck testing of the verifier.

Thus, this commit disables SILInstruction::verifyOperandOwnership() when
PrintMessageInsteadOfAssert is enabled. PrintMessageInsteadOfAssert is renamed
to 'IsSILOwnershipVerifierTestingEnabled' in light of its expanded role.

rdar://29791263
…s forwarded @owned aggregates.

I am going to add some negative tests in a forthcoming commit. I just wrote
these locally and wanted to get them into tree.

rdar://29791263
Previously, we would require an end_borrow for guaranteed subobject borrows.
This was not the end design and of course would have triggered asserts since
end_borrow only supports ending borrows of the same type (by design). This
commit finishes the design by:

1. Verifying that subobject borrows do not have an end_borrow. This is done by
asserting that subobject borrows do not have lifetime ending uses.

2. Treating uses of a subobject borrow as normal liveness uses of the base
borrowed object. This means that the verifier will assert if there are
any uses of borrowed subobjects after the base's end_value.

rdar://29791263
@gottesmm
Copy link
Contributor Author

@swift-ci Please smoke test and merge

@gottesmm
Copy link
Contributor Author

Linux test failure isn't me. It is a frontend compiler crasher. Lets try this again...

@gottesmm
Copy link
Contributor Author

@swift-ci Please smoke test linux platform

1 similar comment
@gottesmm
Copy link
Contributor Author

@swift-ci Please smoke test linux platform

@gottesmm gottesmm merged commit ae02e0f into swiftlang:master Jan 22, 2017
@gottesmm gottesmm deleted the more_semantic_sil branch January 22, 2017 19:02
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