Skip to content

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

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.

private[scalalogging] object Scala2LoggerMacro {

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

private[scalalogging] object Scala2LoggerTakingImplicitMacro {

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.

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