-
Notifications
You must be signed in to change notification settings - Fork 326
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
Use IR.STRUCTURE_COMPARATOR
to compare two IR objects
#11178
Conversation
IR.STRUCTURE_COMPARATOR
to compare two IR objects
Got the failure in parallel PR CI run. Need review! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving although I'm a bit wary of the performance impact. Please make sure it doesn't get worse.
return 0; | ||
} | ||
var aCopy = aIr.duplicate(true, false, false, true); | ||
var bCopy = bIr.duplicate(true, false, false, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks expensive if we do it for every IR that we read?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are not doing this for every IR
we read. The code is only performed before exception:
sb.append("Adding at ").append(at).append(" object:\n ");
dumpObject(sb, res);
sb.append("\nbut there already is:\n ");
dumpObject(sb, prev);
sb.append("\nare they equal: ").append(bothObjectsAreTheSame);
is logged or thrown. How frequently that happens? It happens on CI from time to time. I haven't seen it on my computer yet.
Pull Request Description
Fixes #10985 by providing a special
IR.STRUCTURE_COMPARATOR
that removes metadata and diagnostics before comparing twoIR
objects. Inefficient, not fully correct, but enough for the debugging purposes thisprev != null
check was introduced.Checklist
Please ensure that the following checklist has been satisfied before submitting the PR:
Java,