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

define the scala 2 macros in the scala 3 module #415

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

johnduffell
Copy link

@johnduffell johnduffell commented Oct 17, 2023

This PR is a preview of a fix for #317
It needs the fix from scala/scala3#18663 which is in scala 3.4.0 release (and hopefully 3.3.3) in order to compile without an error Something's wrong: missing original symbol for type tree

It is a fresh implementation although along the same lines at this demonstration PR #350 by @srollins-lucid

It could probably do with some more tests - the current tests show that everything (still) works in all supported versions, but we could add some tests to make sure it works as intended from scala 2 and 3 with the scala 3 dependency only, as per https://docs.scala-lang.org/scala3/guides/migration/tutorial-macro-mixing.html

SBT is probably my weakest point, so I'm happy to believe that I've somehow broken the cross build or the release process during my hacking, so feedback or updates would be appreciated.

@johnduffell johnduffell changed the title define the scala 2 macros in the scala 3 module DO NOT MERGE: define the scala 2 macros in the scala 3 module Oct 17, 2023
build.sbt Outdated
)
incOptions := incOptions.value.withLogRecompileOnMacro(false)
val scala213 = "2.13.12"
val scala3 = "3.4.0-RC1-bin-20231016-69e1338-NIGHTLY" //TODO 3.3.2 or 3.4.0
Copy link
Author

Choose a reason for hiding this comment

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

once a release includes scala/scala3#18663 we can bump this, retest and release

@@ -0,0 +1,85 @@
package com.typesafe.scalalogging
Copy link
Author

Choose a reason for hiding this comment

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

this file is essentially a rename, with minor changes Scala2LoggerMacro to avoid a name clash

@@ -54,4 +55,87 @@ trait LoggerImpl {
inline def trace(inline marker: Marker, inline message: String, inline cause: Throwable): Unit = ${LoggerMacro.traceMessageCauseMarker('underlying, 'marker, 'message, 'cause)}
inline def trace(inline marker: Marker, inline message: String, inline args: Any*): Unit = ${LoggerMacro.traceMessageArgsMarker('underlying, 'marker, 'message, 'args)}
inline def whenTraceEnabled(inline body: Unit): Unit = ${LoggerMacro.traceCode('underlying, 'body)}

//scala 2
Copy link
Author

Choose a reason for hiding this comment

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

the rest is a copy of the scala 2 macro definitions to be compiled in scala 3. Then these can be used from either scala 3 or 2.


type LoggerContext = blackbox.Context { type PrefixType = Logger }
type LoggerContext = blackbox.Context
Copy link
Author

Choose a reason for hiding this comment

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

this was not needed, and it made a cyclical dependency


type LoggerContext[A] = blackbox.Context { type PrefixType = LoggerTakingImplicit[A] }
type LoggerContext[A] = blackbox.Context
Copy link
Author

Choose a reason for hiding this comment

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

this was not needed, and it made a cyclical dependency

@SethTisue SethTisue marked this pull request as draft October 17, 2023 22:05
@SethTisue SethTisue changed the title DO NOT MERGE: define the scala 2 macros in the scala 3 module define the scala 2 macros in the scala 3 module Oct 17, 2023
@SethTisue
Copy link
Collaborator

3.3.2 and 3.4.0 are on Maven Central (and will be announced once tooling support lands)

@johnduffell
Copy link
Author

thanks @SethTisue - I don't believe the fix has been backported to 3.3.2, I think it's likely to end up in 3.3.3 instead.

I have updated the PR to 3.4.0 which does have the fix, but I wonder if we prefer to target latest LTS as per this recommendation? https://www.scala-lang.org/blog/2022/08/17/long-term-compatibility-plans.html#library-maintainers

@johnduffell johnduffell marked this pull request as ready for review February 16, 2024 21:10
@SethTisue
Copy link
Collaborator

I wonder if we prefer to target latest LTS as per this recommendation?

Yes, if it all possible, though it hurts in this case. Fingers crossed 3.3.3 comes along before too much longer.

I suppose one possibility would be to publish a milestone that requires 3.4, then wait for the fix to land in LTS before doing a final release.

@jxnu-liguobin jxnu-liguobin linked an issue Mar 1, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Logging Macro Doesn't Support Scala2 + Scala3 Simultaneously
2 participants