Bug 74890 - Web Inspector: CodeGeneratorInspector.py: generate anonymous types.
Summary: Web Inspector: CodeGeneratorInspector.py: generate anonymous types.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (Deprecated) (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on: 72861
Blocks:
  Show dependency treegraph
 
Reported: 2011-12-19 15:02 PST by Peter Rybin
Modified: 2019-05-02 16:24 PDT (History)
15 users (show)

See Also:


Attachments
Patch (39.08 KB, patch)
2011-12-19 15:11 PST, Peter Rybin
no flags Details | Formatted Diff | Diff
Sample output (9.95 KB, text/plain)
2011-12-19 15:12 PST, Peter Rybin
no flags Details
Sample output InspectorFrontend.h (138.69 KB, text/plain)
2011-12-19 15:13 PST, Peter Rybin
no flags Details
Patch (40.26 KB, patch)
2011-12-21 06:43 PST, Peter Rybin
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Rybin 2011-12-19 15:02:12 PST
Some types are anonymous in Inspector.JSON.
Provide type-safe API for them too.
Comment 1 Peter Rybin 2011-12-19 15:11:20 PST
Created attachment 119923 [details]
Patch
Comment 2 Peter Rybin 2011-12-19 15:12:06 PST
Created attachment 119924 [details]
Sample output
Comment 3 Peter Rybin 2011-12-19 15:13:23 PST
Created attachment 119926 [details]
Sample output InspectorFrontend.h
Comment 4 Pavel Feldman 2011-12-20 02:09:34 PST
Comment on attachment 119923 [details]
Patch

Looks good. What does it do?
Comment 5 Peter Rybin 2011-12-20 12:03:38 PST
(In reply to comment #4)
> (From update of attachment 119923 [details])
> Looks good. What does it do?

Let me describe it here a bit more.

In general it generates now C++ type for anonymous object types from JSON. It takes a name from the type declaration site, usually a parameter name. This all is explained comments in generated file. Also all generated types now refer to other generated types in setter methods — but this is commented out in generated code for now. All necessary forward declarations are also added. Anonymous enums are also generated, but they are in comments too, because we didn't have solution about form the enums should have in C++ API.

Internally:
The change reorganizes type bindings — a polymorphous "code generator" object is factored out from binding.
A helper class Writer is added to allow generating code with ajustable indentations and to support insertion points where additional code can be inserted retroactively.
ForwardListener class is used for preparing necessary forward declarations.
AdHocTypeContext conception is a speculative abstract class that is needed wherever anonymous type can emerge.
Comment 6 Yury Semikhatsky 2011-12-21 01:01:30 PST
(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 119923 [details] [details])
> > Looks good. What does it do?
> 
> Let me describe it here a bit more.
> 
> In general it generates now C++ type for anonymous object types from JSON. It takes a name from the type declaration site, usually a parameter name. This all is explained comments in generated file. Also all generated types now refer to other generated types in setter methods — but this is commented out in generated code for now. All necessary forward declarations are also added. Anonymous enums are also generated, but they are in comments too, because we didn't have solution about form the enums should have in C++ API.
> 
> Internally:
> The change reorganizes type bindings — a polymorphous "code generator" object is factored out from binding.
> A helper class Writer is added to allow generating code with ajustable indentations and to support insertion points where additional code can be inserted retroactively.
> ForwardListener class is used for preparing necessary forward declarations.
> AdHocTypeContext conception is a speculative abstract class that is needed wherever anonymous type can emerge.

This is a good message to be placed into the ChangeLog entry.
Comment 7 Yury Semikhatsky 2011-12-21 01:19:02 PST
What about typedef'ed types such as BreakpointId and ScriptId? I see them declared but never used afterwards neither in comments nor in code. What prevents you from using them as corresponding parameter types e.g. in the following lines?

void scriptParsed(const String& scriptId, const String& url, int startLine, int startColumn, int endLine, int endColumn, const bool* const isContentScript)
void breakpointResolved(const String& breakpointId, /*PassRefPtr<TypeBuilder::Debugger::Location>*/ PassRefPtr<InspectorObject> location);
Comment 8 Peter Rybin 2011-12-21 06:43:43 PST
Created attachment 120175 [details]
Patch
Comment 9 Peter Rybin 2011-12-21 06:48:22 PST
Yury, you have a very sharp eye. Indeed this typedefs were intended for use as parameter types to make client code more consistent. The answer is that I just didn't have time to do it yet and that's in my TODO. I don't think it's a high-pri task right now.

(In reply to comment #7)
> What about typedef'ed types such as BreakpointId and ScriptId? I see them declared but never used afterwards neither in comments nor in code. What prevents you from using them as corresponding parameter types e.g. in the following lines?
> 
> void scriptParsed(const String& scriptId, const String& url, int startLine, int startColumn, int endLine, int endColumn, const bool* const isContentScript)
> void breakpointResolved(const String& breakpointId, /*PassRefPtr<TypeBuilder::Debugger::Location>*/ PassRefPtr<InspectorObject> location);
Comment 10 WebKit Review Bot 2011-12-22 01:22:12 PST
Comment on attachment 120175 [details]
Patch

Clearing flags on attachment: 120175

Committed r103503: <http://trac.webkit.org/changeset/103503>
Comment 11 WebKit Review Bot 2011-12-22 01:22:17 PST
All reviewed patches have been landed.  Closing bug.