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
Patch (6.69 KB, patch)
2014-05-15 02:46 PDT, Tanay
no flags
Tanay
Comment 1 2014-05-14 03:11:03 PDT
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
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.