New issue
Advanced search Search tips

Issue 644384 link

Starred by 3 users

Issue metadata

Status: WontFix
Owner:
Closed: Jun 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug

Blocked on:
issue 673039

Blocking:
issue 578344


Participants' hotlists:
blink-rename

Show other hotlists

Other hotlists containing this issue:
Hotlist-1


Sign in to add a comment

rewrite_to_chrome_style: Need to fix identifiers in generated code

Project Member Reported by lukasza@chromium.org, Sep 6 2016

Issue description

Example: 
After running the tool, wrapUnique is renamed to WrapUnique, but
gen/blink/platform/v8_inspector/protocol/Protocol.h
and
gen/blink/platform/v8_inspector/protocol/Schema.cpp
keep referring to wrapUnique.  AFAICT these sources are generated via third_party/WebKit/Source/platform/inspector_protocol/CodeGenerator.py

 
I've started a CL at https://codereview.chromium.org/2329463004 with postprocessing that needs to happen after running the tool (and cannot ever be either 1) avoided by tweaking/fixing the tool or 2) preemptively renaming upfront).

Sometimes #2 can be arguably done, but I wonder if it is worth it to split it out (given that we would still need postprocessing for generated code).  Examples:
- Is vs is in macro concatenation
- Renaming of things in the assembly

WDYT?
I think these are just some things that we'll have to do in the one big step. Another example is the things emitted by the IDL compiler: we'll just have to have preemptive patches prepared to land with the big style rewrite. If the preemptive patch grows prohibitively large, we'll revisit breaking this up into chunks somehow.
Blockedon: 673039

Comment 4 by dcheng@chromium.org, Dec 14 2016

Cc: yukishiino@chromium.org peria@chromium.org haraken@chromium.org bashi@chromium.org
Components: Blink>Bindings
Labels: -Pri-2 Pri-1
Status: Available (was: Untriaged)
Let's implement a switch for this in the IDL compiler so we can easily switch from the old-style names to the new-style names in the rename. We won't be able to test it too much, but perhaps we can have run-binding-tests dump out something with the new-style names so we can verify that things look approximately correct.

I'm not sure if we need to change anything other than the bindings generator for the big rename: IIRC, the .in files and various other things of that nature don't depend on non-generated Blink code, so we can probably update them after the rename is done.
I am not sure if a switch would be sufficient - we will also need to tweak Jinja templates (and it would feel slightly awkward to have to choose between different templates based on a switch).  For now, I've been stashing (not yet complete/sufficient) changes to Jinja templates in https://codereview.chromium.org/2329463004.

BTW, note that https://codereview.chromium.org/2329463004 also includes changes to non-IDL code generators - I think some of these process .in files, but I don't remember at this point :-(

Comment 6 by dcheng@chromium.org, Dec 14 2016

I think we just need to add some indirection to make this work: the contexts for attributes and methods needs to include a new key called 'cpp_name', which would be cased accordingly. The switch would control whether cpp_name would be generated with namesLikeThis or NamesLikeThis.

We'd have to make this work with ImplementedAs, but does this sound like something that could work?
Labels: Needs-Feedback
Owner: dcheng@chromium.org
Status: Assigned (was: Available)
dcheng, does this bug still apply? I know there's been a lot of evolution in the thinking about the rewrite, and when it just happened, afaik namesLikeThis were kept.

Comment 8 by dcheng@chromium.org, Apr 10 2017

I think there are some things that we still want to clean up--outside of WebIDL, other generated code should probably using the standard naming conventions.
Is Blink>Bindings still the right component, then?
Status: WontFix (was: Assigned)

Sign in to add a comment