When a @MainActor or lazy global is accessed its value flows through an
unsafeMutableAddressor function that returns Builtin.RawPointer, then
pointer_to_address, then load. The addressor has no self so the existing
nameThroughSelf path returned nothing, leaving the name as 'unknown'.
Fall back to using the callee name from the addressor apply when no self
is present. In a full compilation getDeclRef() resolves the AccessorDecl
back to the VarDecl name (e.g. 'globalKlass'); in SIL unit tests the
function name itself is used as the component.
rdar://179181114
Without this, the name inferrer gave up at init_existential_ref (used when
wrapping a concrete value in `any Protocol`) and region isolation diagnostics
fell back to the generic "unknown pattern" catch-all instead of naming the
variable.
Also suppress the `.some` path component when looking through Optional.some
EnumInst wrapping, since users never write `x.some` in source and the suffix
produced confusing diagnostics like "'negotiator.some' cannot be returned".
Adds a lit test covering the regression case: a value stored to an
`inout sending` closure parameter and returned through an existential.
rdar://178775464
VariableNameInferrer previously failed to infer a name for the result of
a call when the result carried no other debug info. In particular:
- 'try_apply' results were never handled at all: the result is
delivered as the normal successor's block argument, so it has no
defining instruction and never reached the apply-handling code.
- The 'function_ref'-with-self path pushed the callee's *decl context*
name (often '<unknown decl>') rather than the callee's own name.
- A bare call result (no self, no debug info) was left unnamed.
As a result, region-isolation diagnostics such as the 'inout sending'
returned-value error fell through to the catch-all "please file a bug"
unknown-pattern diagnostic for ordinary code like:
func f(mutex: borrowing Mutex<[NS]>) -> [NS] {
mutex.withLock { items in items.filter { _ in true } }
}
Now a name is recovered uniformly for apply / try_apply / begin_apply
results:
- If the callee has a self argument, walk into self and produce
'self.member'.
- Otherwise, name the result after the callee function itself.
The 'filter' example above now produces a clear, named diagnostic
("'items.filter' cannot be returned") instead of the unknown-pattern
error.
Adds naming tests for apply/try_apply/begin_apply results (with and
without self), and updates two move-only diagnostics whose inferred
names improved from 'unknown' to 'self.<accessor>'.
rdar://178534561
Restructure the five "incompatible region merge" diagnostics emitted by
IncompatibleRegionMergeDiagnosticEmitter so that the primary error never
depends on recovering a value name. Previously, when VariableNameInferrer
could not infer a name for either side of the merge, every emitter fell
back to emitUnknownPatternError() and the user got the unhelpful "we do
not understand this pattern" diagnostic for a violation the analysis had
in fact understood.
Now:
* Each primary states the *operation* that performs the merge and the
two isolation domains involved -- e.g. "passing arguments to %kind2
could allow for references between values exposed to %0 and %1 code,
risking data races". The primary requires only the isolation strings
and (where applicable) the callee decl or cast target type.
* Per-value details move to follow-up notes anchored at each value's
*defining* source location (parameter decl for SILFunctionArgument,
defining-instruction loc otherwise) so the user can see where each
side came from. Two note variants: named ("'self.x' is exposed to ...
code") when name inference succeeds, bare ("value is exposed to ...
code") otherwise. We deliberately do not emit a "value of type T"
variant because SIL values often do not carry a reliable AST type.
* The shared emitMergeRegionValueNote() helper centralises note
emission and the new preciseSourceLocForValue() helper picks the
most precise SourceLoc available for a value.
emitNonisolatedFunction now falls through to emitUnknown rather than
emitUnknownPatternError when ApplySite/getCalleeDeclRef recovery fails,
so the primary still fires with isolation information in those cases.
emitUnknownPatternError remains reachable only when isolation info
itself is invalid -- a genuinely unknown pattern.
rdar://175592408
We cannot use spare bits or other overlapping storage layout tricks with fundamentally
address-only enums, and we can take advantage of this to do borrowing switches or other
in-place projections without copying the value. However, for resilient enums, the
implementation may use spare bit packing, but the type must be handled address-only
outside of its defining module, and we didn't have a way to express that with
borrowing switch. Optimization passes have also been running into problems with the
complexity that we were using `unchecked_take_enum_data_addr` sometimes as a pure
operation. This patch splits the instruction into three:
- `unchecked_inplace_enum_data_addr` represents a nondestructive in-place enum
projection. It is only allowed for enums whose projection operation is
nondestructive.
- `unchecked_take_enum_data_addr` represents a destructive enum projection,
invalidating the enum and leaving the payload to be further consumed.
This matches the current instruction's semantics.
- `unchecked_borrow_enum_data_addr` represents a borrowing enum projection.
The instruction takes a second operand for "scratch" space, which the
enum representation may be copied into in order to avoid invalidating the
enum value, so the result is dependent on the lifetime of both the
original enum and the scratch buffer. This allows for borrowing switches
over resilient enums.
`unchecked_borrow_enum_data_addr` is implemented by taking advantage of the
"address-only enums can't do spare bit optimization" property at runtime.
We inspect the operand type's bitwise-borrowability from its metadata. If
the type is bitwise-borrowable, then we are allowed to bitwise-copy the
enum to the scratch space and apply the projection to the scratch space,
preserving the original value. If the type is not bitwise-borrowable, then
we cannot use spare bit optimization in its layout, so we apply the
projection in-place.
Fixes rdar://174952822.
CONTEXT: This code works by building up a stack of SIL values of values that
need to be transformed into a StringRef as part of generating our name path and
then as a second phase performs the conversion of those values to StringRef as
we pop from the stack.
This is the first in a string of commits that are going to refactor
VariableNameUtils so that the stack will only contain StringRef instead of SIL
entities. This will be accomplished by moving the SIL value -> StringRef code
from the combining part of the algorithm (where we drain the stack) to the
construction of the stack.
The reason why I am doing this is two fold:
1. By just storing StringRef into the stack I am simplifying the code. Today as
mentioned above in the context, we gather up the SILValue we want to process and
then just convert them to StringRef. This means that any time one has to add a
new instruction, one has to update two different pieces of code. By trafficking
in StringRef instead, one only has to update one piece of code.
2. I want to add some simple code that allows for us to get names from closures
which would require me to recurse. I am nervous about putting
values/instructions from different functions in the same data structure. Today
it is safe, but it is bad practice. Instead, by just using StringRef in the
stack, I can avoid this problem.
This was causing us to emit old style diagnostics in certain cases.
Just doing a walk through of the diagnostics and fixing issues while I
am here sprucing up sending diagnostics.
rdar://130915737
Although I don't plan to bring over new assertions wholesale
into the current qualification branch, it's entirely possible
that various minor changes in main will use the new assertions;
having this basic support in the release branch will simplify that.
(This is why I'm adding the includes as a separate pass from
rewriting the individual assertions)
I have been using these in TransferNonSendable and they are useful in terms of
reducing the amount of code that one has to type to use this API. I am going to
need to use it in SILIsolationInfo, so it makes sense to move it into
SILOptimizer/Utils.
NFCI.
The reason why we do this is that we want to treat this as a separate value from
their operand since they are the result of defining a new value.
This has a few nice side-effects, one of which is that if a let results in just
a begin_borrow [var_decl], we emit names for it.
I also did a little work around helping variable name utils to lookup names from
applies that are fed into a {move_value,begin_borrow} [var_decl] which then has
the debug_value we are searching for.
This eliminates a bunch of cases where we couldn't infer the name of a variable
and used the type based diagnostic.
It also eliminates an 'unknown' case for move checking.
I am going to reuse this for TransferNonSendable. In the process I made a few
changes to make it more amenable to both use cases and used the current set of
tests that we have for noncopyable types to validate that I didn't break
anything.
Michael Gottesman
Joe Groff
Kavon Farvardin
Meghana Gupta
Anthony Latsis
Andrew Trick
Michael Gottesman
Nate Chandler
Tim Kientzle