search

jonahgraham / protobuf-dt

Automatically exported from code.google.com/p/protobuf-dt
0 stars 0 forks source link

Nested message support in custom options #121

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
What steps will reproduce the problem?
1.  Create a proto file with a nested message option

import "descriptor.proto";

extend google.protobuf.FieldOptions {
  optional MyCustomOptionType myoption = 50000;
}
message MyCustomOptionType {
  optional MyEnum myoptionfield = 1;
  optional MyCustomOptionType nestedoption = 2;
}
enum MyEnum {
  MyEnumLiteral = 1;
}
message SearchResponse {
  repeated group Result = 1 {
    required string url = 2;
    optional string title = 3;
    repeated string snippets = 4 [(myoption)= { nestedoption: { myoptionfield: MyEnumLiteral } }];
  }
  optional string foo = 4 [(myoption)= { nestedoption: { myoptionfield: MyEnumLiteral } }];
}

What is the expected output? What do you see instead?
Syntax errors appear on the nested "{" elements as FieldNotation value is not 
able to derive MessageNotation.

This valid proto file should have no errors.

What version of the product are you using? On what operating system?
1.0.2 Windows 7 x64

Please provide any additional information below.

Solution is a change to Protobuf.xtext

FieldNotation "value" element must be of type ValueRef instead of SimpleRef in 
order to derive nested MessageNotation elements.

FieldNotation:
  (fieldName=QualifiedName | extension?='[' fieldName=QualifiedName ']') ':' value=ValueRef;

...introduction of this change will cause an ambiguity in the grammar that 
prevents it from being LL(*) parsable...

The root cause of this is the alternate paths to reach this recursion through 
the Field element by deriving either Property or Group...

Field:
  Property | Group;

This can be "corrected" by introducing a Syntactic Predicate that suggests to 
Antlr that "Property" should be preferred over "Group" when an ambiguity is 
present.

Field:
  =>Property | Group;

http://en.wikipedia.org/wiki/Syntactic_predicate

The solution provided builds on the changes proposed in
https://code.google.com/p/protobuf-dt/issues/detail?id=111
but is not dependent on them.

No java code changes are necessary.

While the change does not *appear* to affect any existing code (and the 
included test file ensures groups are still working correctly), this change 
does add an additional aspect of complexity to the protobuf.xtext grammar and 
as such has been made it's own issue distinct from 111 so that it can be judged 
accordingly.

Original issue reported on code.google.com by compuwar...@gmail.com on 15 Sep 2011 at 7:48

Attachments:

GoogleCodeExporter commented 9 years ago 
I believe this issue and solution are superseded by 
https://code.google.com/p/protobuf-dt/issues/detail?id=125
and should be closed / rejected if and when that addresses the issues raised 
here.

Original comment by compuwar...@gmail.com on 30 Sep 2011 at 7:00

GoogleCodeExporter commented 9 years ago

Original comment by alr...@google.com on 12 Oct 2011 at 6:05

GoogleCodeExporter commented 9 years ago

Original comment by alr...@google.com on 18 Oct 2011 at 7:58

GoogleCodeExporter commented 9 years ago

Original comment by alr...@google.com on 3 Nov 2011 at 1:06

GoogleCodeExporter commented 9 years ago 
r8acb6a74568f

Original comment by alr...@google.com on 11 Nov 2011 at 10:17

GoogleCodeExporter commented 9 years ago 
r02ee0170ce38

Original comment by alr...@google.com on 11 Nov 2011 at 11:14

GoogleCodeExporter commented 9 years ago 
Fixed as Issue 155. I started working on full support of custom options based 
on bugs filed internally at Google, as I kept working on it I ended fixing this 
issue as well.

r3bc2f7ac4c7c, r96a713437062, r3216303da8e9, reca7b70fcd9a, r2f429441e726, 
r90b9a83fd09c, r12ee03ab3bfb, r5268fff7f929, rc209d861a5b7, rde0f3163a72d, 
r170e19933ee3, rb7bf6780440a, rfdb47648d534, r60501de66435

Original comment by alr...@google.com on 22 Nov 2011 at 1:18