simulacrum icon indicating copy to clipboard operation
simulacrum copied to clipboard

Ops doesn’t always import the typeclass instance when it needs to.

Open sellout opened this issue 9 years ago • 7 comments

Right now, something like

@typeclass trait Super[A] {
  type T
}
@typeclass trait Sub[A] extends Super[A] {
  def foo(a: A): T
  def bar(a: A): T
}

Will fail to compile because the generated Ops class for Sub[A] doesn’t import the typeclass instance, so it can’t find T. The current implementation only imports the typeclass instance if it finds a type in the immediate trait, not a super-trait.

Which led me to a workaround …

@typeclass trait Super[A] {
  type T
}
@typeclass trait Sub[A] extends Super[A] {
  type T0 = T       // add an alias for the super-trait’s type
  def foo(a: A): T0 // and use it at least once
  def bar(a: A): T
}

Simulacrum now sees that the trait defines a type that is referenced, so it imports the typeclass instance giving access to the super-trait’s type as well.

sellout avatar Apr 06 '16 03:04 sellout

Hmm. This one is a strange issue. I seem to remember that if you extend a trait it should inherit the type aliases - in fact I've made use of this fact in some of my older macros. I should have some time to look at this in the next week to see how simulacrum is doing the expansion with how it differs.

yilinwei avatar May 27 '16 05:05 yilinwei

It used to always import typeClassInstance._ into Ops. Why was this changed?

Jasper-M avatar Jan 31 '17 11:01 Jasper-M

Perhaps they want to avoid some warning for useless import.

ariwaranosai avatar Feb 06 '17 17:02 ariwaranosai

Yes, probably to avoid useless import warnings (aka errors under -Xfatal-warnings.) Unfortunately there is no correct way to import only when it's needed this because simulacrum acts on untypechecked trees. There's no way to know what identifiers you inherit from your parents without knowing who your parents are. It only works in the non-inheritance case by searching the untyped tree for type declarations which match a type reference elsewhere in the class, so even that one is sketchy.

Sad to say, the most straightforward approach which comes to mind is always importing the typeclasses and then using scalafix to elide the useless imports. But after examining scalacenter/scalafix#82, scalacenter/scalafix#83, scalacenter/scalafix#84, the madness of piling hacks upon hacks should be clear enough.

paulp avatar Mar 08 '17 22:03 paulp

A more realistic possibility is to add a third optional argument to @typeclass which lets the programmer specify parents which need to be imported - there's already e.g. @typeclass(excludeParents = List("Show")) so add importParents.

paulp avatar Mar 08 '17 22:03 paulp

Yes, too bad you can’t @SuppressWarnings on Scalac’s own warnings …

sellout avatar Mar 08 '17 22:03 sellout

"Too" "bad"

paulp avatar Mar 08 '17 23:03 paulp