WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
132854
Remove check-webkit-style warnings in JavascriptCore/dfg
https://bugs.webkit.org/show_bug.cgi?id=132854
Summary
Remove check-webkit-style warnings in JavascriptCore/dfg
Tanay
Reported
2014-05-12 23:57:43 PDT
Remove warnings of type : *The parameter name x adds no information, so it should be removed.
Attachments
Patch
(8.51 KB, patch)
2014-05-14 03:11 PDT
,
Tanay
no flags
Details
Formatted Diff
Diff
Patch
(6.69 KB, patch)
2014-05-15 02:46 PDT
,
Tanay
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Tanay
Comment 1
2014-05-14 03:11:03 PDT
Created
attachment 231440
[details]
Patch
Darin Adler
Comment 2
2014-05-14 09:27:02 PDT
Comment on
attachment 231440
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=231440&action=review
Most of the changes look like small improvements, but some of them make things a little worse and should not be done.
> Source/JavaScriptCore/dfg/DFGCapabilities.h:50 > -inline CapabilityLevel capabilityLevel(OpcodeID opcodeID, CodeBlock* codeBlock, Instruction* pc); > +inline CapabilityLevel capabilityLevel(OpcodeID, CodeBlock*, Instruction*);
I am not sure that removing the name “pc” makes the code more clear. I would probably leave it in.
> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:1974 > - void addBranch(const MacroAssembler::JumpList& jump, BasicBlock* destination); > + void addBranch(const MacroAssembler::JumpList&, BasicBlock*);
Removing the name “destination” makes this less clear.
> Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:2243 > + void speculateString(Edge, GPRReg); > + void speculateStringIdentAndLoadStorage(Edge, GPRReg, GPRReg); > + void speculateStringIdent(Edge, GPRReg);
Removing the names “cell”, “string”, and “storage” here makes this less clear. Those names aren’t redundant.
Tanay
Comment 3
2014-05-14 22:37:31 PDT
(In reply to
comment #2
)
> (From update of
attachment 231440
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=231440&action=review
> > Most of the changes look like small improvements, but some of them make things a little worse and should not be done. > > > Source/JavaScriptCore/dfg/DFGCapabilities.h:50 > > -inline CapabilityLevel capabilityLevel(OpcodeID opcodeID, CodeBlock* codeBlock, Instruction* pc); > > +inline CapabilityLevel capabilityLevel(OpcodeID, CodeBlock*, Instruction*); > > I am not sure that removing the name “pc” makes the code more clear. I would probably leave it in. > > > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:1974 > > - void addBranch(const MacroAssembler::JumpList& jump, BasicBlock* destination); > > + void addBranch(const MacroAssembler::JumpList&, BasicBlock*); > > Removing the name “destination” makes this less clear. > > > Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h:2243 > > + void speculateString(Edge, GPRReg); > > + void speculateStringIdentAndLoadStorage(Edge, GPRReg, GPRReg); > > + void speculateStringIdent(Edge, GPRReg); > > Removing the names “cell”, “string”, and “storage” here makes this less clear. Those names aren’t redundant.
Ok, I will omit the changes you mentioned above and submit a patch.
Tanay
Comment 4
2014-05-15 02:46:35 PDT
Created
attachment 231497
[details]
Patch
WebKit Commit Bot
Comment 5
2014-05-19 02:01:30 PDT
Comment on
attachment 231497
[details]
Patch Clearing flags on attachment: 231497 Committed
r169040
: <
http://trac.webkit.org/changeset/169040
>
WebKit Commit Bot
Comment 6
2014-05-19 02:01:32 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug