Skip to content

Conversation

Dan13llljws
Copy link
Contributor

No description provided.

end if

// class body
val whiteList = Set("scala.collection.immutable.Vector.emptyIterator")
Copy link
Contributor

Choose a reason for hiding this comment

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

We should use a Definition here (from dotty.tools.dotc.core.Definitions) instead of a plain String, unless there is some reason not to.

@szymon-rd
Copy link
Contributor

Could you also please add description & problem this PR fixes, as there is no issue attached?

@olhotak
Copy link
Contributor

olhotak commented Mar 18, 2024

This is a step towards fixing #18882 .

case vdef : ValDef if !vdef.symbol.is(Flags.Lazy) && !vdef.rhs.isEmpty =>
val res = eval(vdef.rhs, thisV, klass)
val sym = vdef.symbol
var res = if (whiteList.contains(sym)) Bottom else eval(vdef.rhs, thisV, klass)
Copy link
Contributor

Choose a reason for hiding this comment

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

It can be a val now

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed.

@Dan13llljws
Copy link
Contributor Author

It turns out that adding rootNode in scala.collection.immutable.HashTableBuilder to white list makes all the warnings, including the warnings that was previously supressed by adding emptyIterator in scala.collection.immutable.Vector.

@szymon-rd szymon-rd changed the title added Vector.emptyIterator to white list added HashTableBuilder.rootNode to white list Mar 19, 2024
@szymon-rd
Copy link
Contributor

@liufengyun Could you take a look, please?

end if

// class body
val whiteList = Set(HashSetBuilder_rootNode)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move it to be a field of the class Objects.

Copy link
Contributor

@liufengyun liufengyun left a comment

Choose a reason for hiding this comment

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

I was wondering what if we add the following fields to the whitelist:

Copy link
Contributor

@olhotak olhotak left a comment

Choose a reason for hiding this comment

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

LGTM

@olhotak olhotak enabled auto-merge March 25, 2024 13:21
@olhotak olhotak merged commit 7171211 into scala:main Mar 25, 2024
@Kordyjan Kordyjan added this to the 3.4.2 milestone Mar 28, 2024
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.

5 participants