In `inspector/remote/socket/RemoteInspectorConnectionClient.h` an interface was defined with empty implementation. It is method to be overwritten by sub classes so that parameter name is important. This empty method generates warning. The subclasses of the class are just two and one has it's own implementation. It's good to define this as an abstract method and move default implementation to that sub class.
Created attachment 371765 [details] PATCH
Comment on attachment 371765 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=371765&action=review > Source/JavaScriptCore/inspector/remote/socket/RemoteInspectorConnectionClient.h:40 > + virtual void didAccept(ConnectionID acceptedID, ConnectionID listenerID, Socket::Domain) = 0; Alternative fix is: virtual void didAccept(ConnectionID /* acceptedID */, ConnectionID /* listenerID */, Socket::Domain) { } There’s no need to switch to pure virtual just to get rid of the unused argument warning. Your suggested change is OK if we think it’s better for other reasons.
(In reply to Darin Adler from comment #2) > Alternative fix is: > > virtual void didAccept(ConnectionID /* acceptedID */, ConnectionID /* > listenerID */, Socket::Domain) { } That was also my first approach in downstream. The reason I changed my mind was I cannot be confident with that manner accepted by upstream. I will resend with commenting way. Thanks.
Created attachment 371780 [details] PATCH
Comment on attachment 371780 [details] PATCH r=me, let me know I you need me to cq+
Comment on attachment 371780 [details] PATCH Thanks!
Comment on attachment 371780 [details] PATCH Clearing flags on attachment: 371780 Committed r246299: <https://trac.webkit.org/changeset/246299>
All reviewed patches have been landed. Closing bug.
<rdar://problem/51605252>