close
Skip to content

Make sure ErrorTypes containing type variables are marked as such#7967

Merged
tkremenek merged 1 commit intoswiftlang:swift-3.1-branchfrom
jrose-apple:3.1-to-err-is-human
Mar 8, 2017
Merged

Make sure ErrorTypes containing type variables are marked as such#7967
tkremenek merged 1 commit intoswiftlang:swift-3.1-branchfrom
jrose-apple:3.1-to-err-is-human

Conversation

@jrose-apple
Copy link
Copy Markdown
Contributor

  • Explanation: To make a long story short, we managed to make long-lived types refer to short-lived types; if a new type was reallocated at the exact same address, the original cached long-lived type would be considered "the same" and reused, even though it had the wrong settings. This led to nonsensical errors like 'C.Iterator' is not the same type as 'C.Iterator'. More detail in the commit message.
  • Scope: Unknown. We haven't identified any failures on the 3.1 branch as being a result of this bug, but it wouldn't necessarily be obviously this issue. Even on master it appeared only every hundred runs or so.
  • Issue: rdar://problem/30382791
  • Reviewed by: @DougGregor
  • Risk: Low. The fix marks some more types as "short-lived" by noting that they have type variables inside them. The lifetime change should have no effect, but there's a very small chance that something else in the compiler will not be able to handle the "has type variables" flag being set here. We think it's unlikely, though.
  • Testing: Ran the previously failing compile command 400 times without observing a failure (on the master branch).

…wiftlang#7963)

In some cases, the type checker will produce error types with the
"original type" intact for recovery purposes. Like other types, when
the original type contains a type variable, the ErrorType instance
will be allocated in the "temporary" memory arena associated with the
active constraint solver, because there's no way that particular error
will come up again once the constraint system containing that type
variable has been destroyed.

However, we weren't propagating that "contains a type variable"
information to the newly-created ErrorType, which meant that any type
/containing/ that ErrorType would be allocated in the "permanent"
arena. In practice, this would always be a DependentMemberType; not
too many types are created without looking at their base types at all.
The arena containing the ErrorType would then be deallocated, and its
memory reused later on for a /different/ type. If we ever tried to
make a DependentMemberType whose base was this new type, we'd find the
old DependentMemberType instance in our cache and return that. The
result was that we'd have a DependentMemberType whose "HasError" bit
was set even though the base type was not an error type, and which was
considered canonical whether or not the base type was. This would then
either hit an assertion later on or result in nonsensical errors like
"'C.Iterator' is not the same type as 'C.Iterator'".

Because the reused address always referred to a valid type, none of
the usual dynamic analysis tools could catch the problem. It really
comes down to using a pointer address as a key in a map---but even
without that, we were allocating types in the permanent arena that
really should be temporary, which is a waste of memory.

Likely fixes rdar://problem/30382791, a nondeterministic failure we've
been seeing for weeks on the bots and on developer machines.
@jrose-apple jrose-apple added this to the Swift 3.1 milestone Mar 7, 2017
@jrose-apple
Copy link
Copy Markdown
Contributor Author

@swift-ci Please test

@tkremenek tkremenek merged commit 00b540b into swiftlang:swift-3.1-branch Mar 8, 2017
@jrose-apple jrose-apple deleted the 3.1-to-err-is-human branch March 8, 2017 18:06
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.

2 participants