Skip to content

[move-function] Implement support for checking in defers #40775

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 8 commits into from
Jan 9, 2022

Conversation

gottesmm
Copy link
Contributor

@gottesmm gottesmm commented Jan 8, 2022

This is an initial version of the code needed to handle reinitializing a variable in a defer. There is a bunch of boiler plate since:

  1. I had to reimplement some new data flow analyses on the defer itself.
  2. I had to teach the rest of the code how to use the summarized information from the defer when it was performing normal data flow on the caller.
  3. I had to implement a cloner that converted the inout_aliasable arguments from the defer to our parameters since we will be passing in the moved argument uninitialized in the caller and otherwise memory lifetime invariants are broken.

I am doing some initial testing here and I am going to add more tests once this gets in tree.

…I need to store them.

Specifically, I need to perform the initial gathering of uses for all addresses
at the same time so I can handle their closures all at the same time.
…t_aliasable defer parameters to out parameters after move analysis.
Returns true if this SILFunction must be a defer.

NOTE: This may return false for defer statements that have been
deserialized without a DeclContext. This means that this is guaranteed to
be correct for SILFunctions in Raw SIL that were not deserialized as
canonical. Thus one can use it for diagnostics.
This dataflow is used to determine if a closure/defer use of a value as an
inout_aliasable value is consume inclusive-or liveness requiring use in a
caller. This information is used to determine if a closure invocation or defer
at the source level acts as a use after move in the source level caller of a
moved var or if the moved var is reinited in the function.

This is done by:

1. Classifying all uses of the inout_aliasable argument as either init, consume,
   use.

2. We then perform two separate dataflows that summarize the effect of the
   closure upon the specific address argument:

   a. First for each individual use, we walk upwards along predecessors to the
      beginning of the function stopping at consumes, inits, and other uses. If
      our dataflow eventually reaches the entrance block, then we know that we
      have a use that should be an error. If all uses are blocked by a consume,
      init, or other use then we know that if our operand was moved in a caller,
      the address is not used after the move at least in this callee.

   b. With that in hand, we then perform a similar dataflow for each individual
      consume operation except that we only stop at init blocks and other
      consume blocks. This leaves us with a set of consuming operations of the
      address that can be considered to be consume up out of the callee. We then
      make sure that these consumes form a post dominating set of upon the
      address or bail. If we bail, we do not handle the move in the caller
      causing us to emit an unhandled move error later after all diagnostics
      have run.

After performing both of these dataflows, our caller function can use our
outputs to emit an error if we had a use and a move is live in the caller or
create a new callee where our address argument is converted from an
inout_aliasable parameter to an out parameter.
This converts a set of closure inout_aliasable arguments to be out arguments and
fixes up the initial post dominating consume set of these arguments since we are
hoisting the destroy out of the cloned closure.
…o that we can test end<->end.

Since this is the final in the series of commits, I also added a bunch of tests
since now we have an end to end implementation that /can/ actually be tested.
continue;
}

convertMemoryReinitToInitForm(pairedInst);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NOTE: In the next commit in this PR, I change this to use the cloning logic. This was just a stop gap (that actually emits incorrect code due to the lack of conversion to the out parameter). So if one is reading commit by commit, check the next commit.

@gottesmm
Copy link
Contributor Author

gottesmm commented Jan 8, 2022

@swift-ci test

@gottesmm
Copy link
Contributor Author

gottesmm commented Jan 9, 2022

I discovered that I forgot to implement the multi-block case in the caller of the closures. I have prepared another PR that fixes that and adds more tests in that area and some other areas as well (casts, etc).

@gottesmm gottesmm merged commit 9370929 into swiftlang:main Jan 9, 2022
@gottesmm gottesmm deleted the move-function-defer branch January 9, 2022 02:47
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