Add test case for crash triggered in `swift::NewMangling::Mangler::appendIdentifier(...)`.
Current number of unresolved compiler crashers: 21 (5389 resolved)
/cc @eeckstein - just wanted to let you know that this crasher caused an assertion failure for the assertion `!isDigit(ident[Pos]) && "first char of sub-string may not be a digit"` added on 2016-12-02 by you in commit 684092d7 :-)
Assertion failure in [`include/swift/Basic/ManglingUtils.h (line 187)`](https://github.com/apple/swift/blob/master/include/swift/Basic/ManglingUtils.h#L187):
```
Assertion `!isDigit(ident[Pos]) && "first char of sub-string may not be a digit"' failed.
When executing: void swift::NewMangling::mangleIdentifier(Mangler &, llvm::StringRef) [Mangler = swift::NewMangling::Mangler]
```
Assertion context:
```
// Mangle the sub-string up to the next word substitution (or to the end
// of the identifier - that's why we added the dummy-word).
// The first thing: we add the encoded sub-string length.
M.Buffer << (Repl.StringPos - Pos);
assert(!isDigit(ident[Pos]) &&
"first char of sub-string may not be a digit");
do {
// Update the start position of new added words, so that they refer to
// the begin of the whole mangled Buffer.
if (WordsInBuffer < M.Words.size() &&
M.Words[WordsInBuffer].start == Pos) {
```
Stack trace:
```
0 0x0000000003512118 llvm::sys::PrintStackTrace(llvm::raw_ostream&) (/path/to/swift/bin/swift+0x3512118)
1 0x0000000003512856 SignalHandler(int) (/path/to/swift/bin/swift+0x3512856)
2 0x00007f97fd01b3e0 __restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x113e0)
3 0x00007f97fb749428 gsignal /build/glibc-Qz8a69/glibc-2.23/signal/../sysdeps/unix/sysv/linux/raise.c:54:0
4 0x00007f97fb74b02a abort /build/glibc-Qz8a69/glibc-2.23/stdlib/abort.c:91:0
5 0x00007f97fb741bd7 __assert_fail_base /build/glibc-Qz8a69/glibc-2.23/assert/assert.c:92:0
6 0x00007f97fb741c82 (/lib/x86_64-linux-gnu/libc.so.6+0x2dc82)
7 0x0000000000efb1ec void swift::NewMangling::mangleIdentifier<swift::NewMangling::Mangler>(swift::NewMangling::Mangler&, llvm::StringRef) (/path/to/swift/bin/swift+0xefb1ec)
8 0x0000000000efa72d swift::NewMangling::Mangler::appendIdentifier(llvm::StringRef) (/path/to/swift/bin/swift+0xefa72d)
9 0x0000000000dcef51 swift::NewMangling::ASTMangler::appendDeclName(swift::ValueDecl const*) (/path/to/swift/bin/swift+0xdcef51)
10 0x0000000000dcbbc6 swift::NewMangling::ASTMangler::appendEntity(swift::ValueDecl const*, llvm::StringRef, bool) (/path/to/swift/bin/swift+0xdcbbc6)
11 0x0000000000dccb0e swift::NewMangling::ASTMangler::mangleGlobalVariableFull[abi:cxx11](swift::VarDecl const*) (/path/to/swift/bin/swift+0xdccb0e)
12 0x00000000006694b5 swift::Lowering::SILGenModule::getSILGlobalVariable(swift::VarDecl*, swift::ForDefinition_t) (/path/to/swift/bin/swift+0x6694b5)
13 0x000000000063a3e5 swift::Lowering::SILGenFunction::emitInitializationForVarDecl(swift::VarDecl*) (/path/to/swift/bin/swift+0x63a3e5)
14 0x000000000063c0a1 swift::ASTVisitor<(anonymous namespace)::InitializationForPattern, void, void, void, std::unique_ptr<swift::Lowering::Initialization, std::default_delete<swift::Lowering::Initialization> >, void, void>::visit(swift::Pattern*) (/path/to/swift/bin/swift+0x63c0a1)
15 0x000000000063b164 swift::Lowering::SILGenFunction::emitPatternBinding(swift::PatternBindingDecl*, unsigned int) (/path/to/swift/bin/swift+0x63b164)
16 0x000000000063b2dd swift::Lowering::SILGenFunction::visitPatternBindingDecl(swift::PatternBindingDecl*) (/path/to/swift/bin/swift+0x63b2dd)
17 0x000000000061e0ac swift::Lowering::SILGenModule::visitTopLevelCodeDecl(swift::TopLevelCodeDecl*) (/path/to/swift/bin/swift+0x61e0ac)
18 0x000000000061e96b swift::Lowering::SILGenModule::emitSourceFile(swift::SourceFile*, unsigned int) (/path/to/swift/bin/swift+0x61e96b)
19 0x000000000061f95b swift::SILModule::constructSIL(swift::ModuleDecl*, swift::SILOptions&, swift::FileUnit*, llvm::Optional<unsigned int>, bool, bool) (/path/to/swift/bin/swift+0x61f95b)
20 0x000000000061ffb5 swift::performSILGeneration(swift::FileUnit&, swift::SILOptions&, llvm::Optional<unsigned int>, bool) (/path/to/swift/bin/swift+0x61ffb5)
21 0x000000000047e7a5 swift::performFrontend(llvm::ArrayRef<char const*>, char const*, void*, swift::FrontendObserver*) (/path/to/swift/bin/swift+0x47e7a5)
22 0x000000000043ade7 main (/path/to/swift/bin/swift+0x43ade7)
23 0x00007f97fb734830 __libc_start_main /build/glibc-Qz8a69/glibc-2.23/csu/../csu/libc-start.c:325:0
24 0x0000000000438229 _start (/path/to/swift/bin/swift+0x438229)
```
Fixes assertion failures in SILGen and the optimizer with this
exotic setup:
protocol P {
associatedtype T : Q
}
protocol Q {
func requirement<U : P>(u: U) where U.T == Self
}
Here, we only have a U : P conformance, and not Self : Q,
because Self : Q is available as U.T : Q.
There were three problems here:
- The SIL verifier was too strict in verifying the generic signature.
All that matters is we can get the Self parameter conformance, not
that it's the first requirement, etc.
- GenericSignature::getSubstitutionMap() had a TODO concerning handling
of same-type constraints -- this is the first test-case I've found
that triggered the problem.
- GenericEnvironment::getSubstitutionMap() incorrectly ignored
same-type constraints where one of the two types was a generic
parameter.
Fixes <https://bugs.swift.org/browse/SR-3321>.
Previously all of the following would strip off varying amounts of
MetatypeType, LValueType, InOutType, DynamicSelfType, etc:
- ConstraintSystem::performMemberLookup()
- ConstraintSystem::lookupMember()
- TypeChecker::lookupMember()
- DeclContext::lookupQualified()
- Type::getContextSubstitutions()
The problem is that the higher level methods that took a lookup type
would call the lower level methods, and post-process the result using
the given lookup type. Since different levels of sugar were stripped,
it made the code hard to reason about and opened up edge cases, eg
if a DynamicSelfType or InOutType appears where we didn't expect it.
Since filtering out static/instance and mutating/nonmutating members
is done at higher levels, there's no reason for these name lookup
operations to accept anything other than nominal types, existentials
and archetypes.
Make this so with assertions, and deal with the fallout.
- TypeMatchVisitor doesn't like seeing ErrorTypes -- stop if we have one.
- Common logic for replacing type parameters and variables with
UnresolvedType.
- Fix crash in coerceToType() if one of the two types has an UnresolvedType
in it, but not at the top level.
Fixes one compiler_crasher and some regressions with upcoming patches.
We might allow protocols inside non-generic class/struct/enum
declarations eventually; there's no conceptual difficulty, just
some IRGen and Serialization work that has to happen first.
Also, this fixes a crasher :-)
We "fake" a conformance of UnresolvedType to any protocol.
Instead of returning a concrete conformance, return an
abstract conformance. The concrete conformance had several
problems leading to further crashes:
- The DC was set to a module, not a type declaration context,
since there is not type declaration context here.
- The conformance was marked complete even though it was missing
inherited conformances.
When inserting a new value, assert that there's no existing
value at that key, and to avoid the assert firing when
opening up UnboundGenericTypes, put those replacements in
their own map. We don't need them later.
If the 'override' attribute appears on an invalid decl,
it is removed at the start of typeCheckDecl(). However if
the decl is nested inside a multi-statement closure which
is invalid for other reasons, we may have validated it
but not type checked it, in which case we don't want to
crash in the verifier.
Now, the following are both true or both false:
- AFD->getDeclContext()->isTypeContext()
- AFD->getImplicitSelfDecl() != nullptr
Also, if var->isSelfParameter() returns true, then it is also
the case that var == var->getDeclContext()->getImplicitSelfDecl().
In a generic signature like <T, U where T.A == U>, there is only
one primary archetype, 'T'. 'U' will not appear in SubstitutionMaps,
and the SubstitutionMap version of Type::subst() cannot handle an
interface type containing 'U'.
For interface types, there are two levels of canonicalization:
- The AST-level getCanonicalType() which strips away sugar
- The GenericSignature-level getCanonicalTypeInContext() which
replaces each interface type in an equivalence class with a
representative
SILFunctionTypes must be canonical with respect to a generic
signature.
When emitting re-abstraction thunks we could end up constructing
a SILFunctionType containing 'U', because we were calling
getCanonicalType() on the result of mapTypeOutOfContext().
Fix this by making sure to use getCanonicalTypeInContext() on
the result of mapTypeOutOfContext(), just as we do in capture
lowering.
Unfortunately I worry this problem will come up again in a
different form -- a more general fix would change
mapTypeOutOfContext() to always return "generic signature
canonical" types, and also fix SubstitutionMaps to understand
same type constraints better.
Fixes <https://bugs.swift.org/browse/SR-3326>.
Protocol methods returning optionals, metatypes and functions
returning 'Self' are now handled correctly in Sema when
accessed with a base of existential type, eg:
protocol Clonable {
func maybeClone() -> Self?
func cloneMetatype() -> Self.Type
func getClonerFunc() -> () -> Self
}
let c: Clonable = ...
let _: Clonable = c.maybeClone()
let _: Clonable.Type = c.cloneMetatype()
Previously, we were unable to model this properly, for
several reasons:
- When opening the function type, we replaced the return
value in openedFullType _unconditionally_ with the
existential type. This was broken if the return type
was not the 'Self' parameter, but rather an optional,
metatype or function type involving the 'Self' parameter.
- It was also broken because we lost information; if the
return type originally contained an existential, we had
no way to draw the distinction. The 'hasArchetypeSelf()'
method was a hint that the original type contained a
type parameter, but it was inaccurate in some subtle cases.
- Since we performed this replacement on both the
'openedFullType' and 'openedType', CSApply had to "undo"
it to replace the existential type with the opened
existential archetype. This is because while the formal
type of the access returns the existential type, in
reality it returns the opened type, and CSApply has to
insert the correct ErasureExpr.
This was also done unconditionally, but even if it were
done more carefully, it would do the wrong thing because
of the 'loss of information' above.
- There was something fishy going on when we were dealing
with a constructor that was declared on the Optional type
itself, as seen in the fixed type_checker_crasher.
- TypeBase::eraseOpenedExistential() would transform a
MetatypeType of an opened existential into a MetatypeType
of an existential, rather than an ExistentialMetatypeType
of the existential type.
The new logic has the following improvements:
- When opening a function type, we replace the 'Self' type
parameter with the existential type in 'openedType', but
*not* 'openedFullType'. This simplifies CSApply, since it
doesn't have to "undo" this transformation when figuring
out what coercions to perform.
- We now only use replaceCovariantResultType() when working
with Self-returning methods on *classes*, which have more
severe restrictions than protocols. For protocols, we walk
the type using Type::transform() and handle all the cases
properly.
- Not really related to the above, but there was a whole
pile of dead code here concerning associated type references.
Note that SILGen still needs support for function conversions
involving an existential erasure; in the above Clonable example,
Sema now models this properly but SILGen still crashes:
let _: () -> Clonable = c.getClonerFunc()
Fixes <rdar://problem/28048391>, progress on <rdar://problem/21391055>.
When we have a value 'p' of type 'P.Type' for some protocol 'P',
we require the user to write 'p.init()' rather than 'p()' to
construct an instance of a concrete type conforming to 'P'.
If they write 'p()', we suggest a fixit to insert '.init',
however if there is also a subsequent member access on the
result, for example 'p().foo', we would crash.
Another invalid case that used to crash is when you construct
a protocol metatype directly and then access a member of it,
eg. 'P().foo'.
Be sure to lower the payload type of UnsafeMutablePointer and friends
before converting them, because the Clang type converter expects
optionals to have lowered payloads already.
Also, remove the FunctionType path; with the above change AST-level
function types should no longer show up here.
Fixes <https://bugs.swift.org/browse/SR-2702>.
Currently if destination is unresolved instead of trying to re-typecheck
it again and diagnose structural problems which led to such outcome, it
gets completely ignored in favor of trying to type-check source without
contextual type. That leads to missed diagnostic opportunities, which
results in problems on AST verification and SIL generation stages, and
generally missleading errors e.g. `.x = 0`.
Resolves: SR-3506.
Invalid ASTs like the one in 28625 cause Parse to generate an AST that
looks a heck of a lot like a constructor call, however this makes no
sense as the type in question is a TupleType and lookup asserts in such
cases. Guard against this so we don't crash diagnosing.
Recursive references of computed properties are allowed, but warned
about. Closures `var`s built like the one in 28396 should crash at
runtime, not crash the compiler.
This used to cause SILGen to capture and subsequently load `self` as a
constant. Then, when the super call was SILGen’d, it assumed that
`self` would be loaded Boxed. Diagnose before hitting SILGen so we
don’t have to pollute Lowering with code that handles `self` in this
odd position.
Crashers fixed are minor logic errors:
Patterns: Crash occurred when requesting the range of a created
Pattern. Validity of the range should be checked before returning it
to keep the entire range valid or invalid but never both.
ParseExpr/ParsePattern: The same fixes as the ones provided in #6319
CSDiag: The generic visitor needn’t look through TypeVarTypes either.
