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

Support signed/unsigned/fixed-width integers #37

Merged
merged 2 commits into from Jan 13, 2020

Conversation

cb372
Copy link
Member

@cb372 cb372 commented Jan 10, 2020

Support signed/unsigned/fixed-width integers via tagged types

  • Define Signed, Unsigned, Fixed tags
  • Add PBScalarValueReader/Writers for tagged Int/Long types
  • Write unit tests and extend the property-based tests
  • Document the feature in the README

Initially I planned to implement this using annotations on fields, e.g. @pbFixed, but it didn't really work. e.g. the following two examples would be impossible to encode in Scala using field annotations:

message Foo {
  map<sint32, int> mapWithSignedKeys = 1;
}
message Foo {
  oneof signedOrNormalInt {
    sint32 signed = 1;
    int32 normal = 2;
  }
}

* Define tags
* Add PBScalarValueReader/Writers for tagged Int/Long types
* Write unit tests and extend the property-based tests
* Document the feature in the README
@cb372 cb372 force-pushed the signed-unsigned-fixed-integers branch from 293783d to 276bbd9 Compare January 10, 2020 16:01
@cb372 cb372 changed the title Requesting a pull to 47deg:master from 47deg:signed-unsigned-fixed-integers Support signed/unsigned/fixed-width integers Jan 10, 2020
Copy link

@fedefernandez fedefernandez left a comment

Choose a reason for hiding this comment

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

💯

@@ -78,6 +79,16 @@ trait LowPriorityPBScalarValueWriterImplicits {

trait PBScalarValueWriterImplicits extends LowPriorityPBScalarValueWriterImplicits {

implicit object ContravariantWriter extends Contravariant[PBScalarValueWriter] {

Choose a reason for hiding this comment

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

nice

override def canBePacked: Boolean = true
override def read(input: CodedInputStream): Int @@ Unsigned =
tag[Unsigned](input.readUInt32())
}

Choose a reason for hiding this comment

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

Wondering if we could have some object or abstract class that would receive the types and the function and will create the PBScalarValueReader for us

def apply[A, B](read: CodedInputstream => A): PBScalarValueReader[A @@ B] = ???

@cb372 cb372 merged commit 690bd51 into master Jan 13, 2020
@cb372 cb372 deleted the signed-unsigned-fixed-integers branch January 13, 2020 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants