Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code generation error when transforming protobuf generated classes #43

Closed
wienczny opened this issue Oct 13, 2015 · 11 comments
Closed

Code generation error when transforming protobuf generated classes #43

wienczny opened this issue Oct 13, 2015 · 11 comments
Assignees
Labels

Comments

@wienczny
Copy link

When trying to transform the classes generated using protobuf an error message is issued for every message:

'prefix5' has no member named 'reflectable_generated_main_library#_ReadonlyEnvelope'.

This message is repeated for every _Readonly* class generated.
(see dart-archive/dart-protoc-plugin#51 for an initial description)

@eernstg
Copy link
Collaborator

eernstg commented Oct 13, 2015

Thanks! Do you have code which will reproduce the problem? (I couldn't see any references to code at dart-archive/dart-protoc-plugin#51, and I'm not sure git clone https://github.com/dart-lang/dart-protoc-plugin will do).

@eernstg eernstg added the bug label Oct 13, 2015
@eernstg eernstg self-assigned this Oct 13, 2015
@eernstg
Copy link
Collaborator

eernstg commented Oct 13, 2015

Tried

...$ git clone 'https://github.com/dart-lang/dart-protoc-plugin'
...$ cd dart-protoc-plugin
...$ pub get
...$ pub build --mode=debug test

where the latter produces 14919 lines of diagnostic messages, but it ends in 'Build failed.', none of the messages mention _Readonly, and the 'build' directory tree is empty, so that wasn't enough.

Could you give some more input on how you can reproduce the problem?

@wienczny
Copy link
Author

It took some time to prepare a minimal example: https://github.com/wienczny/ReflectableBugExample
dart-protoc-plugin used is dart-archive/dart-protoc-plugin@58a031c
The bug can be triggered inside the ReflectableBugExample by running pub build

@eernstg
Copy link
Collaborator

eernstg commented Oct 13, 2015

Got it, thanks a lot!

@eernstg
Copy link
Collaborator

eernstg commented Oct 13, 2015

Ah, sorry -- this is a very simple issue: The problem is that there is an attempt to access a private declaration in 'messages.pb.dart' and reflectable does not support access to private declarations.

It is common for Dart tools to react to this situation (access to private declarations across library boundaries) by claiming that the requested name does not exist. The underlying reason is that the private names are renamed such that there is no way to access them (not even with dynamic invocation).

Couldn't you simply make _ReadonlyEnvelope public?

@eernstg eernstg removed the bug label Oct 13, 2015
@eernstg
Copy link
Collaborator

eernstg commented Oct 13, 2015

Duplicate of #33.

@eernstg eernstg closed this as completed Oct 13, 2015
@wienczny
Copy link
Author

If you mean manually modifying all _Readonly* in the generated code - I could do that.
I just prefer a solution that worked across rebuilds, though.

@eernstg
Copy link
Collaborator

eernstg commented Oct 13, 2015

And you don't have enough control over that code generation process to avoid the generation of that as a private name in the first place?

For instance, if there is a transformer somewhere which will consistently create this type of problem for many of its users then the maintainer of that transformer might be persuaded to make that change..

@sigurdm
Copy link
Collaborator

sigurdm commented Oct 14, 2015

We can sort private classes away when generating the code, I guess that is better than generating invalid code - not all classes might be in the users control to make public.

When sorting away a private member it could be logged with logger.fine so a pub build --verbose would reveal that it happened.

@sigurdm sigurdm reopened this Oct 14, 2015
@eernstg eernstg added the bug label Oct 14, 2015
@eernstg
Copy link
Collaborator

eernstg commented Oct 14, 2015

Reclassified as a bug: I forgot that we are actually generating that private name, which shouldn't happen in any case.

eernstg added a commit that referenced this issue Oct 15, 2015
Subtype/superclass quantification can include private classes, even
though this may not be needed (or even intended) by the programmer who
wrote the capabilities, or the programmer who use them via an import.
This CL ensures that we do not any more generate type expressions that
fail to compile because they attempt to access a private class in a
different library.

We keep the private classes in place because they are needed during
traversals of superclass chains (they should not fail when superclass
quantification is enabled), and they do have `declarations` and other
features as expected for any class mirror.

It is quite common that a given "official" class `C` is used by
clients (in type annotations etc.), but they will actually be using
instances of "secret" subclasses, possibly private, often because
they have obtained those objects using factory constructors (a good
example is `List`). This means that it is very useful to be able to
obtain an instance mirror for such an instance, and also to do
`.type` and get a class mirror for it.

That feature can only be supported if we can recognize that a given
object is an instance of a specific private class, and we do not
have a maintainable way to do that. So we leave out that bit for now.

Fixes the code generation problem reported in issue #43.

R=sigurdm@google.com

Review URL: https://codereview.chromium.org/1391013008 .
@eernstg
Copy link
Collaborator

eernstg commented Oct 15, 2015

We're now avoiding to create the expressions that will not compile because they attempt to access a private declaration in a different library (commit 2d9b6f5 on reflectable).

Looking at your explanation again, it looks like the private class was not annotated directly by you, it may instead have been selected by subtype quantification in a reflector that you hadn't even written, and the class itself might even have been generated. That would of course make it quite difficult to avoid that it gets included. But the above-mentioned update should help you, and it does eliminate the problem in the example you provided.

@eernstg eernstg closed this as completed Oct 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants