protoc: inconsistent source locations emitted for options

[jhump]

What version of protobuf and what language are you using?
Version: 3.5.1
Language: N/A (descriptor output of protoc)

What operating system (Linux, Windows, ...) and version?
Linux and OS X

What runtime / compiler are you using (e.g., python version or gcc version)
N/A

What did you do?
Compiled the following to a descriptor w/ source info:

// p.proto
option go_package = "foo";

message M {
  repeated int32 f = 1 [packed = true];
}

Command:

protoc --include_source_info p.proto -o p.protoset

What did you expect to see/What did you see instead?

The source locations in the resulting descriptor include a path indicating the file's options field: [8]. This is in addition to the path for the actual specific option: [8,11]. This makes sense per the doc on the SourceCodeInfo.location field in descriptor.proto: https://github.com/protocolbuffers/protobuf/blob/master/src/google/protobuf/descriptor.proto#L759

However, the field option is given no such treatment. It does not include a path for the field's options field, which would be [4,0,2,0,8]. It does of course include the path to the specific option: [4,0,2,0,8,2]. This is surprising, not only based on the doc comment linked above, but also because it is inconsistent with the treatment of file options.

(Note that v3.5.1 actually shows all options as uninterpreted options in source info, but I know that is already fixed in newer versions by #4342.)

[acozzette]

@jhump Thanks for the bug report. Feel free to send me a pull request if you have a fix.

[jhump]

I think I may have been wrong on this one. I think I misinterpreted some output I was looking at.

It looks like protoc intentionally does not emit separate locations for each field option, all with a path that references the options field. It instead emits only one for that path, and its span corresponds to all options, from [ to ]. So I was expecting it to look like file options, but it doesn't because fields use a compact syntax for defining options.

I'll test a little more and make sure the output makes sense. I'd expect it to have this behavior for enum value and extension range options, too. Whereas all other options (file, message, enum, one of, and service) can be spread around, so should be treated like file options.