google / google/protobuf.dart

Remove or document field set implementations for extensions and unknown fields

Open
#687 2 comments 2 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

question
Dominant language
Dart
Stars
572
Forks
196
Avg merge
1h 59m
Merged PRs (30d)
2

Description

Currently we have 3 field set implementations:

I don't understand why we need 3 different implementations and it's not documented. Conceptually we just need to map integers (field tags) to proto values. As far as I understand, the only difference between a known field and an unknown field is that for unknown fields FieldInfo won't be available. We don't need this much code for this.

We will probably need a new value class (like PbMap, PbList) for unknown length-delimited values (strings, bytes, messages, repeated fields). The representation will just be Uint8List.

I'm not sure if we need to support groups, but we will need another value class for unknown groups, with Uint8List to store the group contents.

When we need to check whether the field for a tag is known, unknown, or extension, we could have Set<int> unknownFields and Set<int> extensionFields in _FieldInfo.

One question is how to merge two unknown groups or length-delimiteds. Merging values is not specified in proto spec. We have two options:

  • Collect values of merged groups and length-delimited stuff. This means PbUnknownLengthDelimited (or whatever we want to call it) will store List<Uint8List> instead of just Uint8List, and add to the list as we merge values. Same for groups.

    This is currently what we do in UnknownFieldSet.

  • Override the current value. This is simpler.

If I'm missing something and we really need 2 extra classes and a few hundred extra lines and tests then we should document why this is needed.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Read protobuf/lib/src/protobuf/field_set.dart, unknown_field_set.dart, extension_field_set.dart, and field_info.dart to understand why the three implementations differ. Compare their handling of known, unknown, extension, length-delimited, and group fields. Done means either a documented rationale for the existing design or an agreed, tested simplification.

Written by the indexing model from the issue text.

Assessment

Tech stack
dart
Domain
api
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.