I also removed the -verify-sil-ownership flag in favor of a disable flag
-disable-sil-ownership-verifier. I used this on only two tests that still need
work to get them to pass with ownership, but whose problems are well understood,
small corner cases. I am going to fix them in follow on commits. I detail them
below:
1. SILOptimizer/definite_init_inout_super_init.swift. This is a test case where
DI is supposed to error. The only problem is that we crash before we error since
the code emitting by SILGen to trigger this error does not pass ownership
invariants. I have spoken with JoeG about this and he suggested that I fix this
earlier in the compiler. Since we do not run the ownership verifier without
asserts enabled, this should not affect compiler users. Given that it has
triggered DI errors previously I think it is safe to disable ownership here.
2. PrintAsObjC/extensions.swift. In this case, the signature generated by type
lowering for one of the thunks here uses an unsafe +0 return value instead of
doing an autorelease return. The ownership checker rightly flags this leak. This
is going to require either an AST level change or a change to TypeLowering. I
think it is safe to turn this off since it is such a corner case that it was
found by a test that has nothing to do with it.
rdar://43398898
I have been meaning to do this change for a minute, but kept on putting it off.
This describes what is actually happening and is a better name for the option.
This just eliminates -enable-sil-ownership from all target-swift-frontend and
target-swift-emit-silgen RUN lines. Both of those now include
enable-sil-ownership in their expansion.
For a resilient protocol that has defaulted associated types, emit
default associated conformance witnesses that compute associated
conformances based on that default witness.
This completes the implementation of resilience protocols that
add new, defaulted associated types, rdar://problem/44167982.
Most of this patch is just removing special cases for materializeForSet
or other fairly mechanical replacements. Unfortunately, the rest is
still a fairly big change, and not one that can be easily split apart
because of the quite reasonable reliance on metaprogramming throughout
the compiler. And, of course, there are a bunch of test updates that
have to be sync'ed with the actual change to code-generation.
This is SR-7134.
The SILGen testsuite consists of valid Swift code covering most language
features. We use these tests to verify that no unknown nodes are in the
file's libSyntax tree. That way we will (hopefully) catch any future
changes or additions to the language which are not implemented in
libSyntax.
I am going to leave in the infrastructure around this just in case. But there is
no reason to keep this in the tests themselves. I can always just revert this
and I don't think merge conflicts are likely due to previous work I did around
the tooling for this.
Otherwise, the plus_zero_* tests will have plus_zero_* as a module name, causing
massive FileCheck problems.
The reason why I am doing it with the main tests is so that I can use it when
syncing branches/etc.
radar://34222540
We used to give witness thunks public linkage if the
conforming type and the protocol are public.
This is completely unnecessary. If the conformance is
fragile, the thunk should be [shared] [serialized],
allowing the thunk to be serialized into callers after
devirtualization.
Otherwise for private protocols or resilient modules,
witness thunks can just always be private.
This should reduce the size of compiled binaries.
There are two other mildly interesting consequences:
1) In the bridged cast tests, we now inline the witness
thunks from the bridgeable conformances, which removes
one level of indirection.
2) This uncovered a flaw in our accessibility checking
model. Usually, we reject a witness that is less
visible than the protocol; however, we fail to
reject it in the case that it comes from an
extension.
This is because members of an extension can be
declared 'public' even if the extended type is not
public, and it appears that in this case the 'public'
keyword has no effect.
I would prefer it if a) 'public' generated a warning
here, and b) the conformance also generated a warning.
In Swift 4 mode, we could then make this kind of
sillyness into an error. But for now, live with the
broken behavior, and add a test to exercise it to ensure
we don't crash.
There are other places where this "allow public but
ignore it, kinda, except respect it in some places"
behavior causes problems. I don't know if it was intentional
or just emergent behavior from general messiness in Sema.
3) In the TBD code, there is one less 'failure' because now
that witness thunks are no longer public, TBDGen does not
need to reason about them (except for the case #2 above,
which will probably require a similar workaround in TBDGen
as what I put into SILGen).
We can get the generic signature from the generic environment
now, and for generic subscript protocol witnesses, using the
signature of the conformance is wrong; it won't have the
generic parameters of the subscript itself.
Also, emit the materializeForSet callback in the right place in
SILModule. Instead of adding it at the end, put it before the
materializeForSet itself. This makes tests a bit easier to write.
Textual SIL was sometimes ambiguous when SILDeclRefs were used, because the textual representation of SILDeclRefs was the same for functions that have the same name, but different signatures.
Textual SIL was sometimes ambiguous when SILDeclRefs were used, because the textual representation of SILDeclRefs was the same for functions that have the same name, but different signatures.
This flag is hopefully going away one day, and using it for testing
resilience is especially suspect. Just invoke the frontend directly
to build the necessary modules with -emit-module first.
Allow 'static' (or, in classes, final 'class') operators to be
declared within types and extensions thereof. Within protocols,
require operators to be marked 'static'. Use a warning with a Fix-It
to stage this in, so we don't break the world's code.
Protocol conformance checking already seems to work, so add some tests
for that. Update a pile of tests and the standard library to include
the required 'static' keywords.
There is an amusing name-mangling change here. Global operators were
getting marked as 'static' (for silly reasons), so their mangled names
had the 'Z' modifier for static methods, even though this doesn't make
sense. Now, operators within types and extensions need to be 'static'
as written.
SILGen will now be able to look up the default implementation
corresponding to a requirement while emitting conformances, and
reference the default witness thunk instead of emitting duplicate
thunks for each conforming type.
IRGen continues to only emit resilient defaults if the protocol itself
is resilient; otherwise, it just ignores the default witness table.
Now that we apply the callback with the correct generic signature, the
assert can go away. It was being triggered if the protocol extension was
defined in a different resilience domain; otherwise we prefer direct
access anyway.
Note that materializeForSet now has to be able to re-abstract Self when
invoking the callback, since we might have to go from a thin metatype
to a thick metatype.
We don't really need to mangle the AST type of the callback, besides
it doesn't have an AST type at all, because it is polymorphic.
Also, mangle the closure as if it were parented by the requirement
and not the witness, for consistency with witness thunks.
The mangling should still be unique since it includes the conformance.
NFC other than updating tests for new mangling.
Previously we would emit two types of MaterializeForSet implementations
in SILGen:
- materializeForSet for a concrete storage declaration
- materializeForSet witness thunk in a conformance
This refactoring decouples the code from taking a conformance, which is
needed for two new types of materializeForSet that we need:
- materializeForSet witness thunk in a default witness table -- this is
necessary in order to be able to resiliently add storage requirements
with default implementations to protocols
- materializeForSet vtable thunk -- this is necessary to fix a missing
re-abstraction case with overriding storage in a subclass
This patch brings us closer to implementing these two. For default
implementations, we still have an issue in that the materializeForSet
has a different "generic signature abstraction pattern" in concrete
and default witnesses, so default and concrete witnesses for
materializeForSet are currently ABI-incompatible because the type
metadata for the storage is passed differently to the callback.
This patch wires up SILGenDefaultWitnessTable to actually emit
thunks and add them to the SILDefaultWitnessTable, using the
new logic in Sema for inferring default implementations.
Note that default witness thunks are mangled like the protocol
requirement itself.
After emitting thunks, SILGen populates a SILDefaultWitnessTable
for consumption by IRGen.
Default witness thunks for properties and subscripts are not
supported yet; a bit more refactoring in MaterializeForSet
emission is necessary.
Previously SILDefaultWitnessTables only included "resilient" default
implementations, which are currently defined as those that appear at the
end of a protocol, after any requirements without defaults.
However, this was too inflexible. Instead, include all entries in the
SILDefaultWitnessTable, with invalid entries standing in for requirements
without defaults.
Previously, the minimum witness table size was a separate parameter, also
appearing in SIL syntax; now it can be calculated by looking at the entries
themselves. The getResilientDefaultEntries() method of SILDefaultWitnessTable
returns the same result as getEntries() did previously.
