| Summary: | clang-tidy: Fix unnecessary copy of objects for operator==() methods | ||||||
|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
| Component: | WebCore Misc. | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||
| Status: | RESOLVED FIXED | ||||||
| Severity: | Normal | CC: | aestes, commit-queue, ews-watchlist, keith_miller, mark.lam, msaboff, saam, webkit-bug-importer, wenson_hsieh, ysuzuki | ||||
| Priority: | P2 | Keywords: | InRadar | ||||
| Version: | WebKit Local Build | ||||||
| Hardware: | Unspecified | ||||||
| OS: | Unspecified | ||||||
| Attachments: |
|
||||||
I also ran this terrible hack of a command to try to find other operator==() methods that weren't using const references:
$ grep -r 'bool operator==(const' Source | egrep -v '(&[ \),]| &[a-zA-Z])'
The only interesting things it found were:
Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp: bool operator==(const InterferenceEdge other) const
Source/JavaScriptCore/runtime/JSCJSValue.h:inline bool operator==(const JSValue a, const JSCell* b) { return a == JSValue(b); }
Source/JavaScriptCore/runtime/JSCJSValue.h:inline bool operator==(const JSCell* a, const JSValue b) { return JSValue(a) == b; }
Source/WTF/wtf/text/StringView.h:inline bool operator==(const char* a, StringView b) { return equal(b, a); }
I'll fix AirAllocateRegistersByGraphColoring.cpp in this patch, but handle the others using separate patches.
Created attachment 357340 [details]
Patch v1
(In reply to David Kilzer (:ddkilzer) from comment #2) > I also ran this terrible hack of a command to try to find other operator==() > methods that weren't using const references: > > $ grep -r 'bool operator==(const' Source | egrep -v '(&[ \),]| &[a-zA-Z])' You may also want to look for "!=" > > The only interesting things it found were: > > Source/JavaScriptCore/b3/air/AirAllocateRegistersByGraphColoring.cpp: > bool operator==(const InterferenceEdge other) const > > Source/JavaScriptCore/runtime/JSCJSValue.h:inline bool operator==(const > JSValue a, const JSCell* b) { return a == JSValue(b); } > Source/JavaScriptCore/runtime/JSCJSValue.h:inline bool operator==(const > JSCell* a, const JSValue b) { return JSValue(a) == b; } > > Source/WTF/wtf/text/StringView.h:inline bool operator==(const char* a, > StringView b) { return equal(b, a); } > > I'll fix AirAllocateRegistersByGraphColoring.cpp in this patch, but handle > the others using separate patches. Comment on attachment 357340 [details] Patch v1 Clearing flags on attachment: 357340 Committed r239237: <https://trac.webkit.org/changeset/239237> All reviewed patches have been landed. Closing bug. (In reply to Keith Miller from comment #4) > (In reply to David Kilzer (:ddkilzer) from comment #2) > > I also ran this terrible hack of a command to try to find other operator==() > > methods that weren't using const references: > > > > $ grep -r 'bool operator==(const' Source | egrep -v '(&[ \),]| &[a-zA-Z])' > > You may also want to look for "!=" Thanks! I already did, but didn't find any beyond what I mentioned in Comment #2. :) |
Running `clang-tidy -header-filter=.* -checks='-*,performance-*,-performance-noexcept-*' ...` on WebCore source files found these unnecessary object copies: Source/WebCore/contentextensions/HashableActionList.h:56:46: warning: the const qualified parameter 'other' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param] bool operator==(const HashableActionList other) const ^ & Source/WebCore/contentextensions/HashableActionList.h:61:46: warning: the const qualified parameter 'other' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param] bool operator!=(const HashableActionList other) const ^ & Source/WebCore/platform/network/FormData.h:88:47: warning: the const qualified parameter 'other' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param] bool operator==(const EncodedFileData other) const ^ & Source/WebCore/platform/network/FormData.h:152:47: warning: the const qualified parameter 'other' is copied for each invocation; consider making it a reference [performance-unnecessary-value-param] bool operator==(const EncodedBlobData other) const ^ &