[WIP] Check if opts succeed in CI #2545
Conversation
There was a problem hiding this comment.
Hello, and thank you for opening this PR! 🎉
All contributors have signed the CLA, thank you! ❤️
Commit Messages
We want to keep history, but for that to actually be useful we have
some rules on how to format our commit messages (relevant xkcd).
Please stick to these guidelines for commit messages:
- Separate subject from body with a blank line
- When fixing an issue, start your commit message with
Fix #<ISSUE-NBR>:- Limit the subject line to 72 characters
- Capitalize the subject line
- Do not end the subject line with a period
- Use the imperative mood in the subject line ("Added" instead of "Add")
- Wrap the body at 80 characters
- Use the body to explain what and why vs. how
adapted from https://chris.beams.io/posts/git-commit
Have an awesome day! ☀️
|
@DarkDimius Looks green to me, the failure comes from the bytecode tests I've added (and never tried to run bootstrapped), it's looks like a test infrastructure thing: [error] Test dotty.tools.dotc.SimplifyPosTests.initializationError failed: java.lang.Exception: No runnable methods, took 0.002 sec
[error] Test dotty.tools.dotc.SimplifyNegTests.initializationError failed: java.lang.Exception: No runnable methods, took 0.0 sec
[error] Test dotty.tools.dotc.SimplifyEquivalences.initializationError failed: java.lang.Exception: Test class should have exactly one public constructor, took 0.0 sec |
|
@OlivierBlanvillain could you please modify the bytecode tests to make sure they pass? |
|
|
|
@DarkDimius Does dotty emit mixin forwarders? |
|
@smarter, it’s likely that your suggestion is the right one.
We don’t emit forwarders if default method dispatch will yield the
same result.
…On 27 May 2017, at 16:33, Guillaume Martres wrote:
@DarkDimius Does dotty emit mixin forwarders?
--
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#2545 (comment)
|
|
Doesn't Scala 2.12 always emit forwarders for performance reasons now? |
Applies need to have same receiver :-)
They may have side-effects, unlike reading other arguments.
The situation is a follows: Without this commit `testOnly dotty.tools.dotc.CompilationTests -- *testPickling` fail with more precise types after pickling. Reverting tpd.scala to master breaks the "tree referencial equiality" condition used in Simplify to detect a fix point. For example `vulpix t7336.scala` (this one also requires the Skolem#toString change). This hack is sufficent for Simplify to reach its fix points without breaking pickling... [SQUASH] [TODO] fix tpd .toString hack
3 failing tests left: tests/run/t8933c.scala tests/pos/t348plus.scala dotty1 from idempotency1
This is removing a disparity between lubs obtained before and after erasure. Consider this example: ``` if b ast.Trees.Match(???) else ast.Trees.Literal(???) ``` Before erasure this would be an or type that's later widen to `ast.Trees.TermTree`. Because of `erasedLub` bias for classes, recomputing this lub after erasure lead to `ast.Trees.Tree` instead. This disparity prevents a local optimisation from being Ycheckable.
This also seems to be a reason why Ycheck:front fails. typeAssigner is REMOVING partial evaluation that typer did when you copy a tree. It means that types would be different if you test pickling. Update made to homogenize: - Don't print constant types in pickling. - Sort union types according to their .show String
This is the issue discussed on scala#2439. The fix implemented there (6f3aa3c) breaks a bunch of other tests; to be further investigated.
Make Apply and TypeApply copy only if function or argument types have changed.
|
I'm working on fixing Junit4 in Dotty |
|
@DarkDimius I have a fix for the SimplifyTests, it it ok to test it by force pushing this branch? |
|
@OlivierBlanvillain, yes please force push. I'll still fix JUnit 4 in separate branch. |
OlivierBlanvillain
force-pushed
the
local-opt-time
branch
from
c570f39
to
71f06ef
Compare
|
@DarkDimius it passed the CI! http://dotty-ci.epfl.ch/lampepfl/dotty/2614 I've pushed it to #2513. |
If they do, I'll force push to PR by @OlivierBlanvillain