Skip to content

[SILGen] Don't crash when calling a generic init with default args. #7169

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 1 commit into from
Jan 31, 2017

Conversation

jrose-apple
Copy link
Contributor

In cases where a default value is used for a parameter with generic type, the argument list might be empty. In that case, we don't need to emit any arguments!

Fixes a crash that came up when writing test cases for #7156.

In cases where a default value is used for a parameter with generic
type, the argument list might be empty. In that case, we don't need
to emit any arguments!
@jrose-apple
Copy link
Contributor Author

@swift-ci Please test

@slavapestov
Copy link
Contributor

Why is this specific to generic arguments?

@slavapestov
Copy link
Contributor

Why didn't this happen when calling things other than initializers? I think this warrants further investigation.

@jrose-apple
Copy link
Contributor Author

That's fair, but I'm not qualified to do it. Want a Radar?

@slavapestov
Copy link
Contributor

What was the actual crash you saw?

@jrose-apple
Copy link
Contributor Author

Assertion failed: (!ParamInfos.empty()), function claimNextParameter, file /Volumes/Data/swift-public/swift/lib/SILGen/SILGenApply.cpp, line 2961.
0  swiftc                   0x00000001088847a8 llvm::sys::PrintStackTrace(llvm::raw_ostream&) + 40
1  swiftc                   0x00000001088839c6 llvm::sys::RunSignalHandlers() + 86
2  swiftc                   0x0000000108884df9 SignalHandler(int) + 361
3  libsystem_platform.dylib 0x000000010b4f4fba _sigtramp + 26
4  libsystem_platform.dylib 0x0000ffff00001fa0 _sigtramp + 4105228288
5  libsystem_c.dylib        0x000000010b27bf8b abort + 127
6  libsystem_c.dylib        0x000000010b2446e6 basename_r + 0
7  swiftc                   0x0000000105eb1bfa (anonymous namespace)::ArgEmitter::emit(swift::Lowering::ArgumentSource&&, swift::Lowering::AbstractionPattern) + 5370
8  swiftc                   0x0000000105eb3ad2 (anonymous namespace)::ArgEmitter::emitShuffle(swift::Expr*, swift::Expr*, llvm::ArrayRef<swift::TupleTypeElt>, swift::ConcreteDeclRef, llvm::ArrayRef<swift::Expr*>, llvm::ArrayRef<int>, llvm::ArrayRef<unsigned int>, swift::Type, swift::Lowering::AbstractionPattern) + 4354
9  swiftc                   0x0000000105eb2865 (anonymous namespace)::ArgEmitter::emitExpanded(swift::Lowering::ArgumentSource&&, swift::Lowering::AbstractionPattern) + 2197
10 swiftc                   0x0000000105eb082f (anonymous namespace)::ArgEmitter::emit(swift::Lowering::ArgumentSource&&, swift::Lowering::AbstractionPattern) + 303
11 swiftc                   0x0000000105eafe84 (anonymous namespace)::CallSite::emit(swift::Lowering::SILGenFunction&, swift::Lowering::AbstractionPattern, (anonymous namespace)::ParamLowering&, llvm::SmallVectorImpl<swift::Lowering::ManagedValue>&, llvm::SmallVectorImpl<std::__1::pair<swift::Lowering::LValue, swift::SILLocation> >&, llvm::Optional<swift::ForeignErrorConvention> const&, swift::ImportAsMemberStatus const&) && + 452
12 swiftc                   0x0000000105e9c747 (anonymous namespace)::CallEmission::apply(swift::Lowering::SGFContext) + 5863

@jrose-apple
Copy link
Contributor Author

Really I'm just trying to get #7156 in, because this isn't a regression. :-/

@slavapestov
Copy link
Contributor

That looks like something is not lining up. We should not be claiming a parameter if we don't need to. Go ahead and file a JIRA and assign it to me, I need to rework that code some time soon for generic subscripts anyway.

@jrose-apple
Copy link
Contributor Author

Mmk. What do you want me to do for now? Land this because it fixes a crash? Or take the problematic test case out of #7156 for now?

@jrose-apple
Copy link
Contributor Author

Filed SR-3808.

@slavapestov
Copy link
Contributor

I think landing this for now is OK

@jrose-apple
Copy link
Contributor Author

All right. Thanks for the review, and the push to file the follow-up bug!

@jrose-apple jrose-apple merged commit 59ddb5b into swiftlang:master Jan 31, 2017
@jrose-apple jrose-apple deleted the all-args-defaulted branch January 31, 2017 23:43
@jckarter
Copy link
Contributor

jckarter commented Feb 1, 2017

I agree, FWIW, this looks like a reasonable spot fix for 3.1.

Copy link
Contributor

@jckarter jckarter left a comment

Choose a reason for hiding this comment

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

LGTM

jrose-apple added a commit to jrose-apple/swift that referenced this pull request Feb 1, 2017
…wiftlang#7169)

In cases where a default value is used for a parameter with generic
type, the argument list might be empty. In that case, we don't need
to emit any arguments!
tkremenek pushed a commit that referenced this pull request Feb 1, 2017
…ed initializer (#7171)

* [SILGen] Don't crash when calling a generic init with default args. (#7169)

In cases where a default value is used for a parameter with generic
type, the argument list might be empty. In that case, we don't need
to emit any arguments!

* Fix ParameterList::clone to handle deserialized defaults arguments. (#7156)

It was checking the wrong predicate, and therefore failing to mark
inherited default arguments as actually being inherited.

While here, explicitly clear out default arguments from non-inherited
cloned parameter lists. I don't think this case can come up today, but
it's better to be correct when we do hit it.

rdar://problem/30167924
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.

3 participants