swiftlang / swift

The Swift Programming Language
https://swift.org
Apache License 2.0
67.41k stars 10.34k forks source link

[SR-8096] Dictionary(grouping:by:) with KeyPaths reports an error being thrown and not handled #50629

Open ffried opened 6 years ago

ffried commented 6 years ago
Previous ID SR-8096
Radar rdar://problem/41427336
Original Reporter @ffried
Type Bug

Attachment: Download

Environment $ swift --version Apple Swift version 4.2 (swiftlang-1000.0.16.7 clang-1000.10.25.3) Target: x86_64-apple-darwin17.6.0 Also happens in Swift 4.1 (bundled in Xcode 9.4.1)
Additional Detail from JIRA | | | |------------------|-----------------| |Votes | 0 | |Component/s | Compiler | |Labels | Bug, KeyPaths | |Assignee | None | |Priority | Medium | md5: b1664bc4840f9032925fe6709e7281f1

Issue Description:

The following convenience initializer on Dictionary reports an error being thrown where there isn't any error at all:

extension Dictionary {
   init<S>(grouping values: S, by keyPath: KeyPath<S.Element, Key>) where S: Sequence, Value == [S.Element] {
      self.init(grouping: values) { $0[keyPath: keyPath] }
   }
}

The error from the compiler:

s41_noErrorThrown.swift:3:7: error: call can throw, but it is not marked with 'try' and the error is not handled
      self.init(grouping: values) { $0[keyPath: keyPath] }
      ^
belkadan commented 6 years ago

@jckarter, @rudkx, do you know what's going on here?

jckarter commented 6 years ago

@rjmccall wrote the throws propagation analysis originally. I don't think key path application does anything out of the ordinary here, but maybe there's a visitor that I neglected to add the new AST nodes to?

jckarter commented 6 years ago

@swift-ci create

rjmccall commented 6 years ago

KeyPath application never throws, right? (As long as we never have throwing accessors.) That initializer is `rethrows`, but I would expect that closure to be inferred as non-throwing. Maybe there's something broken about either the `rethrows` analysis or the throwing-ness inference with initializers.

jckarter commented 6 years ago

Yeah, key path application never throws. I wonder whether I failed to communicate that fact somewhere and it's assuming it does throw, though.

rjmccall commented 6 years ago

I don't think the analysis is based on expressions opting out like that anywhere.

rjmccall commented 6 years ago

Yeah, the key path is a red herring; this code type-checks:

func foo<S, Key>(grouping values: S, by keyPath: KeyPath<S.Element, Key>) -> Dictionary<Key, [S.Element]> where S: Sequence {
  return Dictionary(grouping: values) { $0[keyPath: keyPath] }
}

It's almost certainly just the self-rebinding expression nodes interfering with the rethrows analysis.

ffried commented 6 years ago

Can confirm that. It seems to be the self.init.
This code also does not emit any errors:

extension Dictionary {
   init<S>(grouping values: S, by keyPath: KeyPath<S.Element, Key>) where S: Sequence, Value == [S.Element] {
      self = Dictionary(grouping: values) { $0[keyPath: keyPath] }
   }
}