search

p4lang / p4runtime

Specification documents for the P4Runtime control-plane API
Apache License 2.0
146 stars 89 forks source link

semantics of negative bitwidths #310

Open blp opened 4 years ago

blp commented 4 years ago

Multiple places in the P4Runtime specification use 32-bit signed integer fields for bit widths. Is this a mistake? Are there intended semantics for negative bit widths?

antoninbas commented 4 years ago

I was pretty sure we discussed this at a WG meeting a while back. I had to dig deep, but I did eventually find a discussion pre-dating the 1.0 release, in which we decided to stick to using unsigned integers there. This is the link to the minutes (September 26th 2018): https://docs.google.com/document/d/1yMOzmIOJo4_q2yCc1_eGcniag-Q7w2F_f3kKu9JB64c/edit#heading=h.qraa5mhgicfr

And here is a copy of the relevant part of the transcript:

Protobuf best practices: uint vs int? Antonin: we are consistently using int32 - and not uint32 - for “unsigned” integer entities (e.g. length, bitwidth…), why? Tom: after doing some research internally, it seems that it is mostly for historical reasons; some languages (e.g. older versions of Java) did not have native unsigned integer types Konstantin: for C++, Google style guide advises against using unsigned integer types Antonin: probably “safer” to use int32 (easier to catch errors in the server) Decision: stick with current choice and prefer int32 to uint32

We should probably consider documenting this in the spec, and we can possibly revisit it for P4Runtime 2.0 if needed.

blp commented 4 years ago

Thanks for the explanation!