Refactor field-descriptor

[bkuehnert]

This PR makes the following changes to the field-descriptor class.

1. The TYPE slot has been repurposed. Instead of storing a string that named the protobuf type, it now stores the base lisp type for the field. "Base" here means that if a type is repeated, then TYPE is bound to [type] rather than (list-of [type]). This is because this information is duplicated by the label and container fields anyway.
2. The CLASS slot has been repurposed. Instead of storing the lisp type (if it is an enum, map, or message) or a keyword (if it's scalar), it now stores a keyword which indicates if the type is an enum, map, message, or scalar. This is useful since we often need to distinguish between these different classes of types, and previously this was impossible at compile time.
3. DEFAULT has been removed and replaced with INITFORM. Previously default held either a default or a symbol which indicated if the default was an empty list or vector. This was confusing and unnecessary, and it required re-computing a type's default several times.
4. CONTAINER has been added as a slot that stores either :LIST, :VECTOR, or NIL. This indicates which type of lisp container is used for a repeated field (or NIL if the field is not repeated). This information is required by the deserializer, and was previously pulled out of the DEFAULT slot.
5. SET-TYPE has been removed. It previously stored the lisp type.
6. LISP-TYPE has been removed. It didn't do anything with the removal of aliased types.

Apologies in advance for the giant PR.

[Slids]

Great work, made a few comments

[bkuehnert]

I have to say, this PR is too big. Please try and make them a bit smaller. In this case, for example, the slot renamings could have been in their own PR.

By renaming, you mean the edits I did to the :default slot and :initform slot? Those changes were a direct result to the removal of proto-set-type. Additionally, leaving proto-set-type alone for the PR wasn't an option either. I don't remember the exact problem, but due to the protoc changes, I think I would've needed to actually add code to keep proto-set-type to be the same thing (and at that point, it would just carry a near-duplicate of the :type slot).

[cgay]

Weird number of spaces at line-start.  Are you using a 2-space indent?  [whitespace/indent] [3]
    third_party/lisp/cl_protobufs/protoc/field.cc:139
Warning: messages should be named using CamelCase. If this is a legacy file, consider adding "// LINT: LEGACY_NAMES" to the top of the file. [naming_conventions]
    third_party/lisp/cl_protobufs/tests/zigzag-proto.proto:5:8

[Slids]

; file: third_party/lisp/cl_protobufs/define-proto.lisp ; in: DEFMACRO DEFINE-MAP ; (DEFMACRO CL-PROTOBUFS:DEFINE-MAP ; (PROTOBUFS-IMPLEMENTATION::TYPE-NAME ; &KEY PROTOBUFS-IMPLEMENTATION::KEY-TYPE ; PROTOBUFS-IMPLEMENTATION::VAL-TYPE ; PROTOBUFS-IMPLEMENTATION::VAL-KIND ; PROTOBUFS-IMPLEMENTATION::INDEX) ; #<(SIMPLE-BASE-STRING ; 290) Define a lisp type given the data for a protobuf map type. ;
; Parameters: ; TYPE-NAME: Map type name. ; KEY-TYPE: The lisp type of the map's keys. ; VAL-TYPE: The lisp type of the map's values. ; VAL-KI... {100B3EEC0F}> ; (CHECK-TYPE PROTOBUFS-IMPLEMENTATION::INDEX INTEGER) ; (LET ((PROTOBUFS-IMPLEMENTATION::SLOT PROTOBUFS-IMPLEMENTATION::TYPE-NAME) ; (PROTOBUFS-IMPLEMENTATION::NAME ; (PROTOBUFS-IMPLEMENTATION::CLASS-NAME->PROTO ; PROTOBUFS-IMPLEMENTATION::TYPE-NAME)) ; (PROTOBUFS-IMPLEMENTATION::READER ; (LET # ; #)) ; (PROTOBUFS-IMPLEMENTATION::INTERNAL-SLOT-NAME ; (PROTOBUFS-IMPLEMENTATION::FINTERN "%~A" ; PROTOBUFS-IMPLEMENTATION::SLOT)) ; (PROTOBUFS-IMPLEMENTATION::QUAL-NAME ; (PROTOBUFS-IMPLEMENTATION::MAKE-QUALIFIED-NAME ; PROTOBUFS-IMPLEMENTATION::CURRENT-MESSAGE-DESCRIPTOR #)) ; (CLASS (PROTOBUFS-IMPLEMENTATION::FINTERN #)) ; (PROTOBUFS-IMPLEMENTATION::MSLOT ; (PROTOBUFS-IMPLEMENTATION::MAKE-FIELD-DATA :INTERNAL-SLOT-NAME ; PROTOBUFS-IMPLEMENTATION::INTERNAL-SLOT-NAME :EXTERNAL-SLOT-NAME ; PROTOBUFS-IMPLEMENTATION::SLOT :TYPE 'HASH-TABLE :INITFORM '# ; :ACCESSOR PROTOBUFS-IMPLEMENTATION::READER)) ; (PROTOBUFS-IMPLEMENTATION::MFIELD ; (MAKE-INSTANCE 'CL-PROTOBUFS:FIELD-DESCRIPTOR :NAME # :TYPE CLASS ; :KIND :MAP :QUALIFIED-NAME ; PROTOBUFS-IMPLEMENTATION::QUAL-NAME :LABEL :OPTIONAL ; ...)) ; (PROTOBUFS-IMPLEMENTATION::MAP-DESC ; (PROTOBUFS-IMPLEMENTATION::MAKE-MAP-DESCRIPTOR :NAME ; PROTOBUFS-IMPLEMENTATION::NAME ; :KEY-TYPE ; PROTOBUFS-IMPLEMENTATION::KEY-TYPE ; :VAL-TYPE ; PROTOBUFS-IMPLEMENTATION::VAL-TYPE ; :VAL-KIND ; PROTOBUFS-IMPLEMENTATION::VAL-KIND))) ; (PROTOBUFS-IMPLEMENTATION::RECORD-PROTOBUF-OBJECT CLASS ; PROTOBUFS-IMPLEMENTATION::MAP-DESC ; :MAP) ; `(PROGN ; CL-PROTOBUFS:DEFINE-MAP ; PROTOBUFS-IMPLEMENTATION::MAP-DESC ; ((PROTOBUFS-IMPLEMENTATION::RECORD-PROTOBUF-OBJECT ',CLASS ; ,PROTOBUFS-IMPLEMENTATION::MAP-DESC ; :MAP)) ; ,PROTOBUFS-IMPLEMENTATION::MFIELD ; ,PROTOBUFS-IMPLEMENTATION::MSLOT))) ; --> SB-INT:NAMED-DS-BIND SB-INT:BINDING LET* IF ; ==> ; NIL ; ; caught STYLE-WARNING: ; This is not a (MEMBER :SCALAR :MESSAGE :GROUP :ENUM): ; NIL ; See also: ; The SBCL Manual, Node "Handling of Types" ERROR: FAIL: SB-INT:TYPE-STYLE-WARNING 'This is not a (MEMBER :SCALAR :MESSAGE :GROUP :ENUM): NIL See also: The SBCL Manual, Node "Handling of Types"'

[Slids]

Where are we now:

Done: LISP-TYPE has been removed. It didn't do anything with the removal of aliased types. CONTAINER has been added as a slot that stores either :LIST, :VECTOR, or NIL. This indicates which type of lisp container is used for a repeated field (or NIL if the field is not repeated). This information is required by the deserializer, and was previously pulled out of the DEFAULT slot.

I think done: (Needs to be renamed initform) DEFAULT has been removed and replaced with INITFORM. Previously default held either a default or a symbol which indicated if the default was an empty list or vector. This was confusing and unnecessary, and it required re-computing a type's default several times.

TODO

The TYPE slot has been repurposed. Instead of storing a string that named the protobuf type, it now stores the base lisp type for the field. "Base" here means that if a type is repeated, then TYPE is bound to [type] rather than (list-of [type]). This is because this information is duplicated by the label and container fields anyway. The CLASS slot has been repurposed. Instead of storing the lisp type (if it is an enum, map, or message) or a keyword (if it's scalar), it now stores a keyword which indicates if the type is an enum, map, message, or scalar. This is useful since we often need to distinguish between these different classes of types, and previously this was impossible at compile time. SET-TYPE has been removed. It previously stored the lisp type.