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

Allow resolving declarations from parent scope #144

Closed
jatcwang opened this issue Feb 6, 2020 · 7 comments · Fixed by #374
Closed

Allow resolving declarations from parent scope #144

jatcwang opened this issue Feb 6, 2020 · 7 comments · Fixed by #374
Milestone

Comments

@jatcwang
Copy link
Contributor

jatcwang commented Feb 6, 2020

Currently only declarations within the source type is checked. Should we add a config option to increase this scope to parent classes?

@krzemin
Copy link
Member

krzemin commented Mar 10, 2020

Seems like a reasonable feature under a flag (enableParentAccessors?). PRs welcome.

@jatcwang
Copy link
Contributor Author

With #128 being implemented. Do you think it makes sense for the default to be true since it'll only resolve vals from parent scope?

@krzemin
Copy link
Member

krzemin commented Mar 24, 2020

Hmm, I don't have strong opinion. For compatibility reasons we should probably keep it as it was before (don't perform lookup to a parent class). Do you see some other benefits or risks having it enabled/disabled by default?

Other question: how do we handle conflicting members appearing in supertypes, like in this example:

  trait A1 {
    val foo: String = ""
  }

  trait A2 {
    val foo: Int = 0
  }

  class X12 extends A1 with A2
  val x12: X12 = new X12
  x12.foo // : Int
  class X21 extends A2 with A1
  val x21: X21 = new X21
  x21.foo // : String

Will we have to traverse all super types looking for an accessor? This way we would probably have to reimplement Scala's trait linearization, which... may be not worth the effort?
Or maybe compile-time reflection helps us somehow here?

@jatcwang
Copy link
Contributor Author

Thinking about it more, I think not defaulting is true is ok since this usecase is somewhat rare and may cause surprises, it makes sense to specify it explicitly.

I think the compiler already resolves all of that for us by the time we're inspecting members, but I'll have to try it to be sure

@glmars
Copy link

glmars commented Mar 12, 2021

@jatcwang

With #128 being implemented.

Unfortunately, this still doesn't work.

@jatcwang
Copy link
Contributor Author

@glmars #128 doesn't implement this feature it merely fixes a problematic default. Sorry I don't have time to work on this atm but feel free to give it a go.

@MateuszKubuszok
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants