Skip to content

[ownership] Add new pass OwnershipVerifierTextualErrorDumper and use it in ownership verifier FileCheck tests instead of SILVerifier. #31539

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

@gottesmm gottesmm commented May 4, 2020

This will make it easier for me with a few further refactors to make the
ownership verifier testing mode emit per function error numbers instead of the
global error number that it is emitting now.

The reason why this is necessary is that today, the verification by
-sil-verify-all causes the errors to be emitted. That verification is done on a
per value level, rather than a per function level, so it is hard to get per
function error numbers without doing unprincipled things like propagating around
state saying what the current function being verified is.

This pass instead will let me make the error counter be per ErrorBuilder which
are created per function.

One thing to be aware of is that this /will/ cause SILValue::verifyOwnership to
not emit any output when the testing flag is enabled. This is to ensure I only
do not get duplicate textual error messages from the SILVerifier.

…it in ownership verifier FileCheck tests instead of SILVerifier.

This will make it easier for me with a few further refactors to make the
ownership verifier testing mode emit per function error numbers instead of the
global error number that it is emitting now.

The reason why this is necessary is that today, the verification by
-sil-verify-all causes the errors to be emitted. That verification is done on a
per value level, rather than a per function level, so it is hard to get per
function error numbers without doing unprincipled things like propagating around
state saying what the current function being verified is.

This pass instead will let me make the error counter be per ErrorBuilder which
are created per function.

One thing to be aware of is that this /will/ cause SILValue::verifyOwnership to
not emit any output when the testing flag is enabled. This is to ensure I only
do not get duplicate textual error messages from the SILVerifier.
@gottesmm gottesmm requested a review from atrick May 4, 2020 20:59
@gottesmm
Copy link
Contributor Author

gottesmm commented May 4, 2020

This doesn't actually change any output. Its a pure refactor like action.

@gottesmm
Copy link
Contributor Author

gottesmm commented May 4, 2020

@swift-ci smoke test

@gottesmm
Copy link
Contributor Author

gottesmm commented May 4, 2020

@swift-ci test windows platform

@gottesmm gottesmm merged commit 0640f33 into swiftlang:master May 5, 2020
@gottesmm gottesmm deleted the pr-88ca78b6938c98c9da3c1f7a88d81fac297d0ee2 branch May 5, 2020 01:16
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