Skip to content
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

groupByTransform(_).getOrDefault(_, KtList.empty()) crashes always #139

Closed
Ashok-Varma opened this issue May 20, 2021 · 7 comments · Fixed by #140
Closed

groupByTransform(_).getOrDefault(_, KtList.empty()) crashes always #139

Ashok-Varma opened this issue May 20, 2021 · 7 comments · Fixed by #140

Comments

@Ashok-Varma
Copy link

As groupByTransform directly casts KtMutableMap,KtMutableList to KtMap,KtList. when getOrDefault(key, KtList.empty()) is used on returned map from groupByTransform. It crashes with error type 'EmptyList<Currency>' is not a subtype of type 'KtMutableList<Currency>' of 'defaultValue'

Reproducible code

main() {
  var usd = Currency(1, "USD");
  var inr = Currency(2, "INR");
  KtList<Currency> currency = listOf(usd, inr);
  KtList<CurrencyConversionData> conversion =
      listOf(CurrencyConversionData(1, 2));
  KtMap<Currency, KtList<Currency>> conversions =
      _toCurrencyMap(conversion, currency);

  print(conversions.getOrDefault(Currency(1, "USD"), KtList.empty())); // CRASH HERE (type 'EmptyList<Currency>' is not a subtype of type 'KtMutableList<Currency>' of 'defaultValue')
}

KtMap<Currency, KtList<Currency>> _toCurrencyMap(
    KtList<CurrencyConversionData> conversion, KtList<Currency> currency) {
  KtMap<int, Currency> currencyMap = currency.associateBy((cur) => cur.id);
  return conversion.groupByTransform(
    (conv) => currencyMap.get(conv.fromRef)!,
    (conv) => currencyMap.get(conv.toRef)!,
  );
}

class Currency {
  final int id;
  final String ticker;

  Currency(this.id, this.ticker);
}

class CurrencyConversionData {
  final int fromRef;
  final int toRef;

  CurrencyConversionData(this.fromRef, this.toRef);
}
@passsy
Copy link
Owner

passsy commented May 20, 2021

That's an interesting case. I played around with it a bit. It works in pure Kotlin

data class Currency(val id: Int, val ticker: String)

data class CurrencyConversionData(val fromRef: Int, val toRef: Int)

fun _toCurrencyMap(conversion: List<CurrencyConversionData>, currency: List<Currency>): Map<Currency, List<Currency>> {
    val currencyMap: Map<Int, Currency> = currency.associateBy { it.id };
    return conversion.groupBy(keySelector = { conv -> currencyMap[conv.fromRef]!! },
            valueTransform = { conv -> currencyMap[conv.toRef]!! })

}

var usd = Currency(1, "USD")
var inr = Currency(2, "INR")

val currency = listOf(usd, inr)
val conversion: List<CurrencyConversionData> = listOf(CurrencyConversionData(1, 2))
val conversions: Map<Currency, List<Currency>> = _toCurrencyMap(conversion, currency)

conversions.getOrDefault(Currency(1, "USD"), emptyList())

Running it in Dart causes a crash at runtime, which is odd. The code is correct, in your example and in kt.dart. But Dart runs into this bug. While simplifying the code and writing a tiny pure dart sample the error went away. It only appears for nested generics and I could not yet reproduce it.

It's not the first time such errors appear. It might be related to dart-lang/sdk#35518.

There are two ways to resolve this:

  • Explicitly map values in groupByTo, but this comes with a performance hit
  • Create a dart only repro example and hope it gets fixed

@Ashok-Varma
Copy link
Author

Ashok-Varma commented May 21, 2021

Thanks @passsy, Sorry for the big sample, i just cleaned up my usage and posted, It increased your work :p

the following is minimal reproducible sample

  KtMutableMap<String, KtMutableList<String>> mutableAlias =
      linkedMapFrom<String, KtMutableList<String>>();

  KtMap<String, KtList<String>> castedAlias = mutableAlias;
  castedAlias.getOrDefault("dummy_key", KtList.empty());

Even though we are casting to subtype. getOrDefault is somehow checking for actual type and expecting KtMutableList. Does kt.dart implement this getOrDefault ? or is it delegated from dart ?


I'm using below code as workaround. But it's counter intuitive as castedAlias is of type KtMap<K,KtList> and getOrDefault is using KtMutableList

  castedAlias.getOrDefault("dummy_key", KtMutableList.empty());

@Ashok-Varma
Copy link
Author

Ashok-Varma commented May 21, 2021

Here is dart only example (Trimmed kt.dart classes)

main() {
  KtMutableMap<String, KtMutableList<String>> mutableAlias =
      KtMutableMap.empty();
  KtMap<String, KtList<String>> castedAlias = mutableAlias;

  castedAlias.getOrDefault("dummy_key", KtList<String>());
}

abstract class KtMap<K, V> {
  V? getOrDefault(K key, V defaultValue);
}

abstract class KtMutableMap<K, V> implements KtMap<K, V> {
  factory KtMutableMap.empty() => DartMutableMap<K, V>();
}

class DartMap<K, V> extends Object implements KtMap<K, V> {
  DartMap([Map<K, V> map = const {}])
      : _map = Map.unmodifiable(map),
        super();
  final Map<K, V> _map;

  @override
  V? getOrDefault(K key, V defaultValue) => _map[key] ?? defaultValue;
}

class DartMutableMap<K, V> extends Object implements KtMutableMap<K, V> {
  DartMutableMap([Map<K, V> map = const {}])
      : _map = Map<K, V>.from(map),
        super();

  final Map<K, V> _map;

  @override
  V? getOrDefault(K key, V defaultValue) => _map[key] ?? defaultValue;
}

class KtList<T> {
  KtList();
}

abstract class KtMutableList<T> implements KtList<T> {}

Error : type 'KtList<String>' is not a subtype of type 'KtMutableList<String>' of 'defaultValue' which is not true.

@passsy
Copy link
Owner

passsy commented May 21, 2021

Simplified it further and to

void main() {
  final List<int> intList = <int>[];
  final List<num> numList = intList as List<int>;
  numList.add(42.0);
  // Unhandled exception:
  // type 'double' is not a subtype of type 'int' of 'value'
  // #0      List.add (dart:core-patch/growable_array.dart)
  // #1      main (file:///Users/pascalwelsch/Projects/passsy/kt.dart/example/issue_139.dart:4:11)
  // #2      _delayEntrypointInvocation.<anonymous closure> (dart:isolate-patch/isolate_patch.dart:283:19)
  // #3      _RawReceivePortImpl._handleMessage (dart:isolate-patch/isolate_patch.dart:184:12)
}

Which eventually got me to dart-lang/sdk#33841, an issue I upvoted years ago 😁

Also related:

I still have to figure out what it means for this bug. Most likely that I have to add mapping everywhere because dart doesn't support wildcards.

Interestingly in Kotlin:

// works fine 
val charList: List<CharSequence> = listOf<String>("a")
val char: CharSequence = charList.getOrElse(0, { _ -> "qwer"})
println(char) // a
// throws
val mcharList: MutableList<CharSequence> = mutableListOf<String>("a")
val mchar: CharSequence = mcharList.getOrElse(0, { _ -> "qwer"})
println(mchar)

// Type mismatch: inferred type is CharSequence but String was expected
// Type mismatch: inferred type is MutableList<String> but MutableList<CharSequence> was expected

At least it is a compile error and not a runtime crash as with dart.

@passsy
Copy link
Owner

passsy commented May 21, 2021

For exactly this type problem kotlin introduced the @UnsafeVariance annotation, which is used in getOrDefault

@passsy
Copy link
Owner

passsy commented May 21, 2021

I did not use your short sample because the upcast KtMutableList<String> -> KtList<String> which causes such errors is a pure dart issue, not related to kt.dart.

  KtMutableMap<String, KtMutableList<String>> mutableAlias =
      linkedMapFrom<String, KtMutableList<String>>();

  KtMap<String, KtList<String>> castedAlias = mutableAlias;
  castedAlias.getOrDefault("dummy_key", KtList.empty());

What I could solve is that groupBy now returns KtMap<K, KtList<V>> instead of KtMap<K, KtMutableList<V>>

@Ashok-Varma
Copy link
Author

Sounds good @passsy
Thanks for the quick support

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 a pull request may close this issue.

2 participants