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

[SemanticDB] Fix missing symbol occurrence of type params' type bounds for constructor #13284

Merged
merged 4 commits into from
Aug 20, 2021

Conversation

tanishiking
Copy link
Member

@tanishiking tanishiking commented Aug 11, 2021

Fix #13270

This PR fixes missing symbol occurrence of

  • type parameters' upper bounds of constructor 5d3bdf3
  • TypeRef's symbol inside of TypeTree 0687269

For example:

// Missing symbol occurrence of `<: Txn[T]`
trait Txn/*<-_empty_::Txn#*/[T/*<-_empty_::Txn#[T]*/ <: Txn[T]]

The root problem is we are ignoring TypeDef's rhs of type parameters in the constructor. Once it traverses the trees (like TypeBoundsTree and AppliedTypeTree), the original traverser will look into Ident(Txn) and Ident(T) inside of AppliedTypeTree in the following case.

// AST of `trait Txn[T <: Txn[T]]`
TypeDef(
  Txn,
  Template(
    DefDef(
      name <init>,
      paramss = List(
        List(
          TypeDef(
            T,
            TypeBoundsTree(
              TypeTree[TypeRef(ThisType(TypeRef(NoPrefix,module class scala)),class Nothing)],
              AppliedTypeTree(Ident(Txn),List(Ident(T))),EmptyTree
            )
          )
        ),
        List()
      ),
   ...
)

Also registered symbol that appears inside of TypeRef (of TypeTree).
I understand TypeTree can contain any type, but as far as I know, TypeRef is the only type that is present in the source and we need to handle.

@tanishiking tanishiking changed the title Fix missing symbol occurrence of type params' type bounds for constructor Fix missing SemanticDB symbol occurrence of type params' type bounds for constructor Aug 12, 2021
@tanishiking tanishiking changed the title Fix missing SemanticDB symbol occurrence of type params' type bounds for constructor [SemanticDB] Fix missing symbol occurrence of type params' type bounds for constructor Aug 12, 2021
@bishabosha bishabosha self-requested a review August 17, 2021 13:08
@bishabosha bishabosha self-assigned this Aug 17, 2021
trait Obj/*<-_empty_::Obj#*/[T/*<-_empty_::Obj#[T]*/ <: Txn/*->_empty_::Txn#*/[T/*->_empty_::Obj#[T]*/]] extends Elem/*->_empty_::Elem#*/[T/*->_empty_::Obj#[T]*/]

trait Copy/*<-_empty_::Copy#*/[In/*<-_empty_::Copy#[In]*/ <: Txn/*->_empty_::Txn#*/[In/*->_empty_::Copy#[In]*/], Out/*<-_empty_::Copy#[Out]*/ <: Txn/*->_empty_::Txn#*/[Out/*->_empty_::Copy#[Out]*/]] {
def copyImpl/*<-_empty_::Copy#copyImpl().*/[Repr/*<-_empty_::Copy#copyImpl().[Repr]*/[~ <: Txn[~]] <: Elem[~]](in/*<-_empty_::Copy#copyImpl().(in)*/: Repr/*->_empty_::Copy#copyImpl().[Repr]*/[In/*->_empty_::Copy#[In]*/]): Repr/*->_empty_::Copy#copyImpl().[Repr]*/[Out/*->_empty_::Copy#[Out]*/]
Copy link
Member

Choose a reason for hiding this comment

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

it looks like here that there are no occurrences for the parameters of Repr

Copy link
Member

@bishabosha bishabosha Aug 17, 2021

Choose a reason for hiding this comment

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

although looking at the tree it seems maybe something else is hiding the occurrence:

-- Info: /Users/jamie/Workspace/dotty/tests/semanticdb/expect/i9782.scala:9:6 --
9 |  def copyImpl[Repr[~ <: Txn[~]] <: Elem[~]](in: Repr[In]): Repr[Out]
  |  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |here is a defdef tree: DefDef(copyImpl,List(List(TypeDef(Repr,LambdaTypeTree(List(TypeDef(~,TypeBoundsTree(TypeTree[TypeRef(ThisType(TypeRef(NoPrefix,module class scala)),class Nothing)],AppliedTypeTree(Ident(Txn),List(Ident(~))),EmptyTree))),TypeBoundsTree(TypeTree[TypeRef(ThisType(TypeRef(NoPrefix,module class scala)),class Nothing)],AppliedTypeTree(Ident(Elem),List(Ident(~))),EmptyTree)))), List(ValDef(in,AppliedTypeTree(Ident(Repr),List(Ident(In))),EmptyTree))),AppliedTypeTree(Ident(Repr),List(Ident(Out))),EmptyTree)

Copy link
Member

@bishabosha bishabosha Aug 17, 2021

Choose a reason for hiding this comment

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

this could be addressed separately in a new PR

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, right. I think we can fix it in this PR, I'll take a look :)

Copy link
Member Author

@tanishiking tanishiking Aug 20, 2021

Choose a reason for hiding this comment

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

Fixed 5a108df

I found type params are going to this branch:
https://github.com/lampepfl/dotty/blob/28a671a71a3a5f6627a2db0c9cc5d9c0ec6218a2/compiler/src/dotty/tools/dotc/semanticdb/ExtractSemanticDB.scala#L168-L170

and excludeChildren is true if the parameter is HKTypeLambda, and that's why those symbols are missing.

@bishabosha bishabosha dismissed their stale review August 17, 2021 15:03

unrelated to this pr

@bishabosha bishabosha assigned tanishiking and unassigned bishabosha Aug 17, 2021
@bishabosha
Copy link
Member

This will still need a rebase

Copy link
Member

@bishabosha bishabosha left a comment

Choose a reason for hiding this comment

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

Thanks again!

auto-merge was automatically disabled August 20, 2021 10:27

Head branch was pushed to by a user without write access

@tanishiking
Copy link
Member Author

Fixed failed rebase 🙇 28a671a

@bishabosha bishabosha merged commit 28c7d4e into scala:master Aug 20, 2021
@tanishiking tanishiking deleted the missing-occurrence-upperbound branch August 20, 2021 13:48
@Kordyjan Kordyjan added this to the 3.1.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SemanticDB] Missing symbol occurrence for type annotation
3 participants