Skip to content

Supress safe-init warnings for NameKind HashMaps #14918

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

Closed

Conversation

Xavientois
Copy link
Contributor

This adds the @unchecked annotation to each of the four lines that add the object under construction to the relevant HashMap.

In the NameKinds.scala file, all of the NameKinds are created and added to one of four HashMaps based on their superclass. The lines that add the NameKind to the HashMaps are at the end of the relevant superclasses. This leads to a safe-init error since subclasses can add further fields and initialization logic that gets run after these lines.

These warnings could be eliminated without the use of @unchecked, but it would require redundantly adding the same line to all NameKind subclasses that get initialized. This means that anyone who changes this code in the future would need to remember to do this for all of the relevant subclasses. This could also be fixed without @unchecked if there were a mechanism like DelayedInit in Scala3.

Thus, instead of making a more involved change that would render the code redundant, overly verbose, and less maintainable, I opted for using @unchecked instead.

This adds the `@unchecked` annotation to each of the four lines that add the object under construction to the relevant HashMap.

In the `NameKinds.scala` file, all of the `NameKind`s are created and added to one of four HashMaps based on their superclass. The lines that add the NameKind to the HashMaps are at the end of the relevant superclasses. This leads to a safe-init error since subclasses can add further fields and initialization logic that gets run after these lines.

These warnings could be eliminated without the use of `@unchecked`, but it would require redundantly adding the same line to all `NameKind` subclasses that get initialized. This means that anyone who changes this code in the future would need to remember to do this for all of the relevant subclasses. This could also be fixed without `@unchecked` if there were a mechanism like `DelayedInit` in Scala3.

Thus, instead of making a more involved change that would render the code redundant, overly verbose, and less maintainable, I opted for using `@unchecked` instead.
@Xavientois
Copy link
Contributor Author

These are the errors this fixes:

[error] -- Error: /dotty/compiler/src/dotty/tools/dotc/core/NameKinds.scala:155:30
[error] 155 |    qualifiedNameKinds(tag) = this
[error]     |                              ^^^^
[error]     |Cannot prove that the value is fully initialized. Only initialized values may be used as arguments. Calling trace:
[error]     |-> val QualifiedName: QualifiedNameKind = new QualifiedNameKind(QUALIFIED, ".")	[ NameKinds.scala:244 ]
[error]     |                                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]     |-> class QualifiedNameKind(tag: Int, val separator: String)	[ NameKinds.scala:121 ]
[error]     |   ^
[error] -- Error: /dotty/compiler/src/dotty/tools/dotc/core/NameKinds.scala:193:29
[error] 193 |    numberedNameKinds(tag) = this
[error]     |                             ^^^^
[error]     |Cannot prove that the value is fully initialized. Only initialized values may be used as arguments. Calling trace:
[error]     |-> }	[ NameKinds.scala:279 ]
[error]     |    ^
[error]     |-> val UniqueName: UniqueNameKind = new UniqueNameKind("$") {	[ NameKinds.scala:276 ]
[error]     |                                    ^
[error]     |-> case class UniqueNameKind(val separator: String)	[ NameKinds.scala:209 ]
[error]     |   ^
[error]     |-> extends NumberedNameKind(UNIQUE, s"Unique $separator") {	[ NameKinds.scala:210 ]
[error]     |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]     |-> abstract class NumberedNameKind(tag: Int, val infoString: String) extends NameKind(tag) { self => | => s[ NameKinds.scala:173 ]
[error]     |   ^
[error] -- Error: /dotty/compiler/src/dotty/tools/dotc/core/NameKinds.scala:228:33
[error] 228 |    uniqueNameKinds(separator) = this
[error]     |                                 ^^^^
[error]     |Cannot prove that the value is fully initialized. Only initialized values may be used as arguments. Calling trace:
[error]     |-> }	[ NameKinds.scala:279 ]
[error]     |    ^
[error]     |-> val UniqueName: UniqueNameKind = new UniqueNameKind("$") {	[ NameKinds.scala:276 ]
[error]     |                                    ^
[error]     |-> case class UniqueNameKind(val separator: String)	[ NameKinds.scala:209 ]
[error]     |   ^
[error] -- Error: /dotty/compiler/src/dotty/tools/dotc/core/NameKinds.scala:88:27
[error] 88 |    simpleNameKinds(tag) = this
[error]    |                           ^^^^
[error]    |Cannot prove that the value is fully initialized. Only initialized values may be used as arguments. Calling trace:
[error]    |-> val SuperAccessorName: PrefixNameKind = new PrefixNameKind(SUPERACCESSOR, "super$")	[ NameKinds.scala:356 ]
[error]    |                                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |-> class PrefixNameKind(tag: Int, prefix: String, optInfoString: String = "")	[ NameKinds.scala:92 ]
[error]    |   ^
[error]    |-> extends ClassifiedNameKind(tag, if (optInfoString.isEmpty) s"Prefix $prefix" else optInfoString) { | => s[ NameKinds.scala:93 ]
[error]    |           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[error]    |-> abstract class ClassifiedNameKind(tag: Int, val infoString: String) extends NameKind(tag) {	[ NameKinds.scala:75 ]
[error]    |   ^

@olhotak
Copy link
Contributor

olhotak commented Apr 13, 2022

A potential refactoring to try is to split NameKinds into two objects, one with the four caches at the top, and the other with the various vals of specific name kinds at the bottom.

We should improve the error message for promotion failure. (Here, this is warm because its outer is not initialized (Rule 1) but the QualifiedName class does access some fields (namely the cache at the top) of its outer (Rule 2). But the error message doesn't say anything about that.)

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.

2 participants