New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 673406 link

Starred by 1 user

Issue metadata

Status: WontFix
Owner: ----
Closed: Mar 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 582312


Participants' hotlists:
blink-rename


Sign in to add a comment

Trouble with renaming |request| accessor method to |Request|

Project Member Reported by lukasza@chromium.org, Dec 12 2016

Issue description

A naive automated rename will perform the following rename in third_party/WebKit/Source/modules/fetch/Request.h:

-  const FetchRequestData* request() const { return m_request; }
+  const FetchRequestData* Request() const { return request_; }

This will lead to build errors, because now Request is a name of both 1) a type and 2) a method.

 
Cc: esprehn@chromium.org dcheng@chromium.org bashi@chromium.org
I felt that it is worthwhile to open a separate bug from 582312, because here I don't know how to proceed.  In particular - is the "big rename" blocked on tweaking the IDL generator ( issue 673039 ), or is there some sort of a workaround I can do in the meantime?

Some notes:

- Automatic prepending of "Get" prefix won't kick-in here, because the name of the return type is different from the accessor's name

- Manual prepending of "get" prefix is troublesome, because today it would mean having to call "setGetRequest" from non-generated code.  See the diff between patchset #1 and #2 in https://codereview.chromium.org/2573523002.

Comment 2 by dcheng@chromium.org, Dec 12 2016

As a short-term workaround, maybe we could just teach the IDL compiler to drop "Get" when generating the setters.
Summary: Trouble with renaming |request| accessor method to |Request| (was: Trouble with renaming |request| accessor method to |Request| leads to trouble.)
Cc: lukasza@chromium.org nasko@chromium.org
I think this is no longer an issue:
1. We no longer rename method names that are based on IDL
2. I think "request" was not an issue when performing recent dry runs

I don't remember why #2 "healed itself" (probably #1 helped here), but it might be okay to just resolve this bug as WontFix?

Comment 5 by dcheng@chromium.org, Mar 24 2017

Status: WontFix (was: Untriaged)

Sign in to add a comment