JuliaIO / ProtoBuf.jl

Julia protobuf implementation
Other
205 stars 55 forks source link

ERROR: Unsupported type token with semicolon terminated message-expression #211

Closed jaakkor2 closed 2 years ago

jaakkor2 commented 2 years ago

This in file test2.proto

syntax = "proto3";

message MyMessage {
    sint32 a = 1;
    repeated string b = 2;
};

fails with protojl("test2.proto", ".", "output_dir") from ProtoBuf.jl v1.0.3

ERROR: Unsupported type token ProtoBuf.Tokens.Token(ProtoBuf.Tokens.SEMICOLON, ProtoBuf.Tokens.NO_ERROR, (6, 2), (6, 2), "") (SEMICOLON)
Stacktrace:
 [1] error(s::String)
   @ Base .\error.jl:33
 [2] parse_type(ps::ProtoBuf.Parsers.ParserState)
   @ ProtoBuf.Parsers C:\Users\jaakkor2\.julia\packages\ProtoBuf\2r8PN\src\parsing\proto_types.jl:463
 [3] parse_type(ps::ProtoBuf.Parsers.ParserState, definitions::Dict{String, Union{ProtoBuf.Parsers.EnumType, ProtoBuf.Parsers.MessageType, ProtoBuf.Parsers.ServiceType}})
   @ ProtoBuf.Parsers C:\Users\jaakkor2\.julia\packages\ProtoBuf\2r8PN\src\parsing\proto_types.jl:479
 [4] parse_proto_file(ps::ProtoBuf.Parsers.ParserState)
   @ ProtoBuf.Parsers C:\Users\jaakkor2\.julia\packages\ProtoBuf\2r8PN\src\parsing\Parsers.jl:204
 [5] parse_proto_file
   @ C:\Users\jaakkor2\.julia\packages\ProtoBuf\2r8PN\src\parsing\Parsers.jl:164 [inlined]
 [6] _protojl(relative_paths::String, search_directories::String, output_directory::String, options::ProtoBuf.CodeGenerators.Options)
   @ ProtoBuf.CodeGenerators C:\Users\jaakkor2\.julia\packages\ProtoBuf\2r8PN\src\codegen\CodeGenerators.jl:158
 [7] #protojl#34
   @ C:\Users\jaakkor2\.julia\packages\ProtoBuf\2r8PN\src\codegen\CodeGenerators.jl:134 [inlined]
 [8] protojl(relative_paths::String, search_directories::String, output_directory::String)
   @ ProtoBuf.CodeGenerators C:\Users\jaakkor2\.julia\packages\ProtoBuf\2r8PN\src\codegen\CodeGenerators.jl:133
 [9] top-level scope
   @ REPL[26]:1

I saw proto-files in the wild with message-expression ending in a semicolon. libprotoc version 3.21.4 compiles this fine.

Drvi commented 2 years ago

Thanks for reporting this. Yeah, there are still little things like that where we differ from protoc, we'll get it right eventually.:) Should be an easy fix.

dfdx commented 2 years ago

It looks like the fix in #213 doesn't work for tested structures. I'm getting the error on this example from onnx.proto3:

message TensorShapeProto {
  message Dimension {
    oneof value {
      int64 dim_value = 1;
      string dim_param = 2;   // namespace Shape
    };
    // Standard denotation can optionally be used to denote tensor
    // dimensions with standard semantic descriptions to ensure
    // that operations are applied to the correct axis of a tensor.
    // Refer to https://github.com/onnx/onnx/blob/main/docs/DimensionDenotation.md#denotation-definition
    // for pre-defined dimension denotations.
    string denotation = 3;
  };
  repeated Dimension dim = 1;
}

I guess the fix should also be trivial, but I'm not sure what part of the code is responsible for this case.


Julia v1.8 ProtoBuf v1.0.5 https://github.com/JuliaIO/ProtoBuf.jl.git#master

Drvi commented 2 years ago

Thanks for reporting, @dfdx! A fix should be merged soon. Until then, you can just delete the semicolon after the oneof field definition.