Skip to content

Conversation

hamzaremmal
Copy link
Member

No description provided.

@hamzaremmal hamzaremmal self-assigned this Oct 1, 2025
else if denot.isClass && denot.is(Scala2x) || (denot.maybeOwner.lastKnownDenotation.is(Scala2x) && !denot.is(Inline)) then
else if denot.isClass && denot.is(Scala2x)
|| (denot.maybeOwner.lastKnownDenotation.is(Scala2x) && !denot.is(Inline))
|| denot.is(Param) && denot.maybeOwner.is(Method) && denot.maybeOwner.maybeOwner.is(Scala2x) then
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
|| denot.is(Param) && denot.maybeOwner.is(Method) && denot.maybeOwner.maybeOwner.is(Scala2x) then
|| denot.is(Param) && denot.maybeOwner.is(Method) && denot.maybeOwner.maybeOwner.lastKnownDenotation.is(Scala2x) then

to be consistent with the method case?

We might also need to exclude the params of Inline methods? I don't know when that actually happens. What is even an Inline Scala 2 method?

Copy link
Member Author

Choose a reason for hiding this comment

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

We might also need to exclude the params of Inline methods? I don't know when that actually happens.

I actually don't know yet what happened. I was curious too but not had the time yet to investigate. The only thing I know is that it was added by @jchyb.

What is even an Inline Scala 2 method?

For now, I genuinely don't know... but I know that when we will add an inline def in scala.collection.Seq, we will have an inline Scala2 method.

Copy link
Contributor

@jchyb jchyb Oct 1, 2025

Choose a reason for hiding this comment

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

In my defense, this precedes my changes: 6e58d25#diff-7cbbe7d1307696dd5d37afe4ce94c01f85f4945edb2c37b113245ad8c85264c1L30 .
I was only interested in making sure we are not recalculating the denotation, and likely missed the weird nature of the check there. That is to say, I don't know what that is either :(

Copy link
Member Author

Choose a reason for hiding this comment

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

In my defense, this precedes my changes: 6e58d25#diff-7cbbe7d1307696dd5d37afe4ce94c01f85f4945edb2c37b113245ad8c85264c1L30 .

I didn't meant to blame you for this, I only relied on what VSCode showed me for on git blame when I did the change.

@hamzaremmal hamzaremmal marked this pull request as ready for review October 1, 2025 11:18
@hamzaremmal hamzaremmal requested a review from a team as a code owner October 1, 2025 11:18
@hamzaremmal hamzaremmal assigned sjrd and unassigned hamzaremmal Oct 1, 2025
@hamzaremmal hamzaremmal requested a review from sjrd October 1, 2025 11:18
Copy link
Member

@sjrd sjrd left a comment

Choose a reason for hiding this comment

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

OK looks good. There's probably something to investigate further for those "Scala 2 inline" things in the future.

@hamzaremmal
Copy link
Member Author

There's probably something to investigate further for those "Scala 2 inline" things in the future.

There is, I agree.

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 this pull request may close these issues.

3 participants