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

Report duplicates in the IR after each pass #11873

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

JaroslavTulach
Copy link
Member

Pull Request Description

As part of work on #9165 a #11365 PR was created. However it seems that some IR elements are being referenced by multiple parent IR elements and that is causing issues with their metadata being updated multiple times and with different values. This PR is an investigation of what happens, why and by whom?

Your ideas are welcomed although I am going to change the PR state to draft. It is unlikely to get merged in this state anyway.

Important Notes

Checklist

Please ensure that the following checklist has been satisfied before submitting the PR:

  • All code follows the
    Scala,
    Java,
  • Unit tests have been written where possible.

@JaroslavTulach JaroslavTulach added CI: No changelog needed Do not require a changelog entry for this PR. -compiler labels Dec 16, 2024
@JaroslavTulach JaroslavTulach self-assigned this Dec 16, 2024
@JaroslavTulach JaroslavTulach marked this pull request as draft December 16, 2024 10:06
if (reported.containsKey(System.identityHashCode(e))) {
false
} else {
if (idMap.containsKey(e)) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Scans for current ir in preorder and finds first == IR element that appears twice and hasn't yet been reported.

If such a duplFound, then it scans the IR again to find all elements that have the duplFound.orNull among their children() and prints that up.

The results of such a scan are visible in the log for example like:

Duplicate found after org.enso.compiler.pass.desugar.FunctionBinding$: org.enso.compiler.core.ir.Name$Literal@6edf29c1:Named_Pattern
   referenced by org.enso.compiler.core.ir.module.scope.definition.Method$Conversion@3791f50e:Regex.from = (that : Named_Pattern) -> ((compile) Regex ((regex_pattern) that))
   referenced by org.enso.compiler.core.ir.DefinitionArgument$Specified@7c8c70d6:(that : Named_Pattern)

In some cases such a sharing seems innocent, but in others it may be dangerous. Such a dangerous sharing was probably triggered by #11365 - investigation pending.

@JaroslavTulach JaroslavTulach added the CI: Clean build required CI runners will be cleaned before and after this PR is built. label Dec 16, 2024
@@ -50,7 +50,8 @@ case object Imports extends IRPass {
val parts = newName.parts
if (parts.length == 2) {
i.copy(
name = newName.copy(parts = parts :+ mainModuleName),
name =
newName.copy(parts = parts :+ mainModuleName.duplicate()),
Copy link
Member Author

Choose a reason for hiding this comment

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

There was a report:

 Duplicate found after org.enso.compiler.pass.desugar.Imports$: org.enso.compiler.core.ir.Name$Literal@56476c16:Main
   referenced by org.enso.compiler.core.ir.Name$Qualified@76e9f00b:Standard.Base.Main
   referenced by org.enso.compiler.core.ir.Name$Qualified@314b9e4b:Standard.Test.Main

and with bb4aff2 it is gone.

underlying.isLoggable(level)
} catch {
Copy link
Member Author

Choose a reason for hiding this comment

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

There is a runtime-instrument-common/testOnly *ChangesetBuilderTest failure which gets fixed by b4f3973

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
-compiler CI: Clean build required CI runners will be cleaned before and after this PR is built. CI: No changelog needed Do not require a changelog entry for this PR.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant