Skip to content

Do expect/actual for shared code in Deflater#1434

Merged
swankjesse merged 7 commits intomasterfrom
jwilson.0214.deflate_expect_actual
Feb 18, 2024
Merged

Do expect/actual for shared code in Deflater#1434
swankjesse merged 7 commits intomasterfrom
jwilson.0214.deflate_expect_actual

Conversation

@swankjesse
Copy link
Copy Markdown
Collaborator

No description provided.

@swankjesse swankjesse marked this pull request as draft February 15, 2024 03:34
@swankjesse swankjesse changed the title Do expect/actual for shared code in Deflater WIP: Do expect/actual for shared code in Deflater Feb 15, 2024
* limitations under the License.
*/

@file:JvmName("-DeflaterSinkExtensions")
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

extension moved to the multiplatform zlibMain source set

else -> null
}
zStream.avail_in = sourceByteCount.toUInt()
internal val dataProcessor: DataProcessor = object : DataProcessor() {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Super annoying; I can’t extend DataProcessor in Kotlin/Native because that doesn’t match the inheritance hierarchy in Kotlin/JVM.

closed = true
actual constructor() : this(false)

@OptIn(UnsafeNumber::class)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

These should be safe in practice! We don’t expose target-specific numbers in our public API

@swankjesse swankjesse changed the title WIP: Do expect/actual for shared code in Deflater Do expect/actual for shared code in Deflater Feb 15, 2024
@swankjesse swankjesse marked this pull request as ready for review February 15, 2024 04:20
* This class does not offer any partial flush mechanism. For best performance,
* only call [flush] when application behavior requires it.
*/
expect class DeflaterSink
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any reason to make these public? The JVM ones have to remain public for compatibility, but these don't expose any API on top of the underlying Sink.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This API is handy to implement web sockets, SPDY/3, and PNG. We might use it in a future multiplatform Paparazzi!

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm referring specifically to the class. The extension below already provides access to the functionality (although its return type should probably be just Sink).

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I’d like for it to remain public so we keep the option of having APIs like finishDeflate() public. Or APIs to supply a Dictionary mid-compression if that exotic feature is used.

deflater.end()
}

private fun Deflater(nowrap: Boolean) = Deflater(Z_BEST_COMPRESSION, nowrap)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

No way to put this default value onto the expect so you don't need to define this function?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not as far as I know.

> Task :okio:compileKotlinJvm FAILED
e: file:///Volumes/Development/okio/okio/src/jvmMain/kotlin/okio/-JvmPlatform.kt:45:1 Default argument values inside expect declaration 'Deflater' are not allowed if it is actualized via typealias. Possible fix is to remove default argument values in members:
    public constructor Deflater(level: Int = ..., nowrap: Boolean = ...)

@swankjesse swankjesse merged commit e941f18 into master Feb 18, 2024
@swankjesse swankjesse deleted the jwilson.0214.deflate_expect_actual branch February 18, 2024 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants