[ownership] Cleanup how we emit filecheck-compatible verification errors for the ownership verifier. #31372
Conversation
…ors for the ownership verifier. This is doing a few things with a simple change to use a builder: 1. It cleans up how we emit errors so we have a builder object that constructs errors. The errors then just become dumb POD data that the builder vends to callers that via the boolean values describe what errors were found. 2. Now that we have one place where we are actually emitting these errors, I cleaned up how we emit the errors by normalizing the output so function names are quoted the same. 3. I changed our error emission so that we emit a unique count of the errors as we emit them. This makes it so that our pattern matching is much more robust against weird pattern match errors that can be difficult to debug due to the errors having unrelated test cases/file check patterns bleed together. Before/end checks eliminate this problem. I updated all of the relevant test cases. The reason /why/ I am doing this though is that I am going to be adding support to the LinearLifetimeChecker for flagging objects that are outside of the lifetime that we are verifying (meaning either before or after). This is going to cause me to need to track /all/ non consuming uses when performing linear lifetime checks and thus most likely emit more errors. I was finding it to be difficult to update the current tests given the state of the world before this patch, so I was inspired to clean this up to satisfy practical as well as debt concerns.
|
@swift-ci smoke test |
|
Hmm... I may change these tests in a forthcoming commit to use relative error counts in combination with a fixed max error count. |
|
(Will make it easier to update). |
|
Hmmm... I don't know if we have support for that in our current file check impl. May need to wait until the next update from llvm |
|
@atrick The reason why I am numbering is that I want to ensure that I am matching all of the errors that we are emitting. It is important for other uses of the linear lifetime checker that I only handle exactly one error for each bad use. I can't flag the same use multiple times. So I want to control the number/amount carefully. That being said, I am actually going to change it to number per function instead of globally. Otherwise, too difficult to update. |
|
A per-function error number makes sense |
This is doing a few things with a simple change to use a builder:
It cleans up how we emit errors so we have a builder object that constructs
errors. The errors then just become dumb POD data that the builder vends to
callers that via the boolean values describe what errors were found.
Now that we have one place where we are actually emitting these errors, I
cleaned up how we emit the errors by normalizing the output so function names
are quoted the same.
I changed our error emission so that we emit a unique count of the errors as
we emit them. This makes it so that our pattern matching is much more robust
against weird pattern match errors that can be difficult to debug due to the
errors having unrelated test cases/file check patterns bleed
together. Before/end checks eliminate this problem. I updated all of the
relevant test cases.
The reason /why/ I am doing this though is that I am going to be adding support
to the LinearLifetimeChecker for flagging objects that are outside of the
lifetime that we are verifying (meaning either before or after). This is going
to cause me to need to track /all/ non consuming uses when performing linear
lifetime checks and thus most likely emit more errors. I was finding it to be
difficult to update the current tests given the state of the world before this
patch, so I was inspired to clean this up to satisfy practical as well as debt
concerns.