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

Generate generic java signatures #3234

Merged
merged 13 commits into from Oct 30, 2017

Conversation

@Duhemm
Contributor

Duhemm commented Oct 3, 2017

The code has been ported from scalac.

Two flags are added:

  • -Yno-generic-signatures disables the generation of generic signatures
  • -Xverify-signatures runs further verifications of the signatures, and emits a warning if the signature is not valid.

As discussed, the signatures are not generated if they contain symbols that we cannot encode for the JVM.

@dotty-bot

Hello, and thank you for opening this PR! 🎉

All contributors have signed the CLA, thank you! ❤️

Have an awesome day! ☀️

@smarter

This comment has been minimized.

Show comment
Hide comment
@smarter

smarter Oct 3, 2017

Member

test performance please

Member

smarter commented Oct 3, 2017

test performance please

@dotty-bot

This comment has been minimized.

Show comment
Hide comment
@dotty-bot

dotty-bot Oct 3, 2017

performance test scheduled: 1 job(s) in queue, 1 running.

dotty-bot commented Oct 3, 2017

performance test scheduled: 1 job(s) in queue, 1 running.

@dotty-bot

This comment has been minimized.

Show comment
Hide comment
@dotty-bot

dotty-bot Oct 3, 2017

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3234/ to see the changes.

Benchmarks is based on merging with master (432ceac)

dotty-bot commented Oct 3, 2017

Performance test finished successfully:

Visit http://dotty-bench.epfl.ch/3234/ to see the changes.

Benchmarks is based on merging with master (432ceac)

Show outdated Hide outdated compiler/src/dotty/tools/dotc/core/Symbols.scala Outdated
Show outdated Hide outdated compiler/src/dotty/tools/dotc/core/Symbols.scala Outdated
Show outdated Hide outdated compiler/src/dotty/tools/dotc/transform/Erasure.scala Outdated
Show outdated Hide outdated compiler/src/dotty/tools/dotc/core/Symbols.scala Outdated
Show outdated Hide outdated compiler/src/dotty/tools/dotc/core/Symbols.scala Outdated
Show outdated Hide outdated compiler/src/dotty/tools/dotc/transform/Erasure.scala Outdated
Show outdated Hide outdated compiler/src/dotty/tools/dotc/transform/Erasure.scala Outdated
Show outdated Hide outdated compiler/src/dotty/tools/dotc/transform/Erasure.scala Outdated
Show outdated Hide outdated compiler/src/dotty/tools/dotc/transform/Erasure.scala Outdated
Show outdated Hide outdated compiler/src/dotty/tools/dotc/transform/Erasure.scala Outdated
@Duhemm

This comment has been minimized.

Show comment
Hide comment
@Duhemm

Duhemm Oct 10, 2017

Contributor

This is ready for another round of review 😄

Contributor

Duhemm commented Oct 10, 2017

This is ready for another round of review 😄

@@ -0,0 +1,510 @@
package dotty.tools
package dotc

This comment has been minimized.

@allanrenucci

allanrenucci Oct 10, 2017

Member

Should probably be in backend/jvm

@allanrenucci

allanrenucci Oct 10, 2017

Member

Should probably be in backend/jvm

This comment has been minimized.

@odersky

odersky Oct 10, 2017

Contributor

Debatable where it should go. But since most of it is part of erasure I'd leave it in transform.

@odersky

odersky Oct 10, 2017

Contributor

Debatable where it should go. But since most of it is part of erasure I'd leave it in transform.

This comment has been minimized.

@allanrenucci

allanrenucci Oct 10, 2017

Member

It is only called from backend/jvm/DottyBackendInterface.scala. So my reasoning was that this code is platform specific.
But I don't have a strong opinion about where it should be.

@allanrenucci

allanrenucci Oct 10, 2017

Member

It is only called from backend/jvm/DottyBackendInterface.scala. So my reasoning was that this code is platform specific.
But I don't have a strong opinion about where it should be.

/** Helper object to generate generic java signatures, as defined in
* the Java Virtual Machine Specification, §4.3.4
*/
object GenericSignatures {

This comment has been minimized.

@allanrenucci

allanrenucci Oct 10, 2017

Member

GenericJavaSignatures?

@allanrenucci

allanrenucci Oct 10, 2017

Member

GenericJavaSignatures?

@odersky

I did not go through GenericSignatures in detail, since I trust that this was ported correctly from scalac. The integration with the rest of the compiler is now as clean and minimal as it can be, I think. Nice work!

Show outdated Hide outdated compiler/src/dotty/tools/dotc/core/Types.scala Outdated

@allanrenucci allanrenucci added this to the 0.5 Tech Preview milestone Oct 17, 2017

@Duhemm

This comment has been minimized.

Show comment
Hide comment
@Duhemm

Duhemm Oct 23, 2017

Contributor

Rebased and added -Xverify-signatures to the default test options.

Contributor

Duhemm commented Oct 23, 2017

Rebased and added -Xverify-signatures to the default test options.

Duhemm added some commits Sep 29, 2017

Write generic java signatures to classfiles
The code has been ported from scalac.

@allanrenucci allanrenucci merged commit eca989b into lampepfl:master Oct 30, 2017

2 checks passed

CLA User signed CLA
Details
continuous-integration/drone/pr the build was successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment