Closed Bug 1317241 Opened 8 years ago Closed 8 years ago

Run clang-tidy on dom/ module

Categories

(Developer Infrastructure :: Source Code Analysis, defect)

Product:
Developer Infrastructure
Component:
Source Code Analysis
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(firefox53 fixed)

Status:
RESOLVED FIXED
Milestone:
mozilla53
Tracking Flags:
Tracking Status
firefox53 --- fixed

People

(Reporter: andi, Assigned: andi)

References

Details

Attachments

(6 files)

Bug 1317241 - Converts for(...; ...; ...) loops to use the new range-based loops in C++11 in dom/.
58 bytes, text/x-review-board-request
baku
: review+
Details
Bug 1317241 - Use auto type specifier where aplicable for variable declarations to improve code readability and maintainability in dom/.
58 bytes, text/x-review-board-request
baku
: review+
Details
Bug 1317241 - Replace default bodies of special member functions with = default in dom/.
58 bytes, text/x-review-board-request
baku
: review+
Details
Bug 1317241 - Replace string literals containing escaped characters with raw string literals in dom/.
58 bytes, text/x-review-board-request
baku
: review+
Details
Bug 1317241 - Replace integer literals which are cast to bool in dom/.
58 bytes, text/x-review-board-request
baku
: review+
Details
Bug 1317241 - Use C++11's override and remove virtual where applicable in dom/
58 bytes, text/x-review-board-request
baku
: review+
Details
Using the checkers provided by clang-tidy we can refresh the code to make it use the features of C++11 like: - auto variables declarations - default CTORS and DTORS - using new range loop operators We could also break this into sub bugs if there are two many modified files.
Assignee: nobody → bpostelnicu
Attachment #8810349 - Flags: review?(amarchesini)
Attachment #8810350 - Flags: review?(amarchesini)
Attachment #8810351 - Flags: review?(amarchesini)
Attachment #8810352 - Flags: review?(amarchesini)
Attachment #8810353 - Flags: review?(amarchesini)
Attachment #8810354 - Flags: review?(amarchesini)
Comment on attachment 8810349 [details] Bug 1317241 - Converts for(...; ...; ...) loops to use the new range-based loops in C++11 in dom/. https://reviewboard.mozilla.org/r/92696/#review93014 ::: dom/asmjscache/AsmJSCache.cpp:1861 (Diff revision 3) > > bool > ParamTraits<Metadata>::Read(const Message* aMsg, PickleIterator* aIter, > paramType* aResult) > { > - for (unsigned i = 0; i < Metadata::kNumEntries; i++) { > + for (auto & entry : aResult->mEntries) { auto& ::: dom/base/DOMIntersectionObserver.cpp:127 (Diff revision 3) > } > > mRootMargin = value.GetRectValue(); > > - for (uint32_t i = 0; i < ArrayLength(nsCSSRect::sides); ++i) { > - nsCSSValue value = mRootMargin.*nsCSSRect::sides[i]; > + for (auto side : nsCSSRect::sides) { > + nsCSSValue value = mRootMargin.*side; nsCSSValue& ::: dom/base/DOMIntersectionObserver.cpp:127 (Diff revision 3) > } > > mRootMargin = value.GetRectValue(); > > - for (uint32_t i = 0; i < ArrayLength(nsCSSRect::sides); ++i) { > - nsCSSValue value = mRootMargin.*nsCSSRect::sides[i]; > + for (auto side : nsCSSRect::sides) { > + nsCSSValue value = mRootMargin.*side; nsCSSValue& ::: dom/base/nsContentUtils.cpp:7128 (Diff revision 3) > "accept-charset", "accept-encoding", "access-control-request-headers", > "access-control-request-method", "connection", "content-length", > "cookie", "cookie2", "date", "dnt", "expect", "host", "keep-alive", > "origin", "referer", "te", "trailer", "transfer-encoding", "upgrade", "via" > }; > - for (uint32_t i = 0; i < ArrayLength(kInvalidHeaders); ++i) { > + for (auto & kInvalidHeader : kInvalidHeaders) { auto& ::: dom/base/nsContentUtils.cpp:9151 (Diff revision 3) > }; > static mozilla::BloomFilter<12, nsIAtom> sFilter; > static bool sInitialized = false; > if (!sInitialized) { > sInitialized = true; > - for (uint32_t i = 0; i < ArrayLength(nonEscapingElements); ++i) { > + for (auto & nonEscapingElement : nonEscapingElements) { auto& ::: dom/base/nsContentUtils.cpp:9158 (Diff revision 3) > } > } > > nsIAtom* tag = aParent->NodeInfo()->NameAtom(); > if (sFilter.mightContain(tag)) { > - for (uint32_t i = 0; i < ArrayLength(nonEscapingElements); ++i) { > + for (auto & nonEscapingElement : nonEscapingElements) { auto&
Attachment #8810349 - Flags: review?(amarchesini) → review+
Comment on attachment 8810350 [details] Bug 1317241 - Use auto type specifier where aplicable for variable declarations to improve code readability and maintainability in dom/. https://reviewboard.mozilla.org/r/92698/#review93018 ::: dom/base/nsFrameMessageManager.cpp:1845 (Diff revision 3) > uri->GetScheme(scheme); > // We don't cache data: scripts! > if (aShouldCache && !scheme.EqualsLiteral("data")) { > // Root the object also for caching. > - nsMessageManagerScriptHolder* holder = > + auto* holder = > new nsMessageManagerScriptHolder(cx, script, aRunInGlobalScope); maybe this can stay on the same line ::: dom/base/nsFrameMessageManager.cpp:2207 (Diff revision 3) > cb = new SameChildProcessMessageManagerCallback(); > } else { > cb = new ChildProcessMessageManagerCallback(); > RegisterStrongMemoryReporter(new MessageManagerReporter()); > } > - nsFrameMessageManager* mm = new nsFrameMessageManager(cb, > + auto* mm = new nsFrameMessageManager(cb, indentation!
Attachment #8810350 - Flags: review?(amarchesini) → review+
Comment on attachment 8810351 [details] Bug 1317241 - Replace default bodies of special member functions with = default in dom/. https://reviewboard.mozilla.org/r/92700/#review93020 ::: dom/indexedDB/ActorsParent.cpp:337 (Diff revision 3) > bool > HasLiveIndexes() const; > > private: > ~FullObjectStoreMetadata() > - { } > + = default; same line. ::: dom/indexedDB/ActorsParent.cpp:369 (Diff revision 3) > already_AddRefed<FullDatabaseMetadata> > Duplicate() const; > > private: > ~FullDatabaseMetadata() > - { } > + = default; same line
Attachment #8810351 - Flags: review?(amarchesini) → review+
Comment on attachment 8810352 [details] Bug 1317241 - Replace string literals containing escaped characters with raw string literals in dom/. https://reviewboard.mozilla.org/r/92702/#review93022 I like this so much!
Attachment #8810352 - Flags: review?(amarchesini) → review+
Comment on attachment 8810353 [details] Bug 1317241 - Replace integer literals which are cast to bool in dom/. https://reviewboard.mozilla.org/r/92704/#review93024
Attachment #8810353 - Flags: review?(amarchesini) → review+
Comment on attachment 8810354 [details] Bug 1317241 - Use C++11's override and remove virtual where applicable in dom/ https://reviewboard.mozilla.org/r/92706/#review93026 You must work on this patch a bit more. Please, land the patch only when all the extra spaces, and the indentation issues are fixed. ::: dom/base/nsFrameMessageManager.cpp:1950 (Diff revision 5) > - virtual ~SameParentProcessMessageManagerCallback() > + ~SameParentProcessMessageManagerCallback() override > { > MOZ_COUNT_DTOR(SameParentProcessMessageManagerCallback); > } > > - virtual bool DoLoadMessageManagerScript(const nsAString& aURL, > + bool DoLoadMessageManagerScript(const nsAString& aURL, indentation ::: dom/base/nsFrameMessageManager.cpp:2022 (Diff revision 5) > { > MOZ_COUNT_DTOR(ChildProcessMessageManagerCallback); > } > > - virtual bool DoSendBlockingMessage(JSContext* aCx, > + bool DoSendBlockingMessage(JSContext* aCx, > const nsAString& aMessage, indentation ::: dom/base/nsFrameMessageManager.cpp:2050 (Diff revision 5) > } > return cc->SendRpcMessage(PromiseFlatString(aMessage), data, cpows, > IPC::Principal(aPrincipal), aRetVal); > } > > - virtual nsresult DoSendAsyncMessage(JSContext* aCx, > + nsresult DoSendAsyncMessage(JSContext* aCx, indentation ::: dom/base/nsFrameMessageManager.cpp:2110 (Diff revision 5) > - virtual ~SameChildProcessMessageManagerCallback() > + ~SameChildProcessMessageManagerCallback() override > { > MOZ_COUNT_DTOR(SameChildProcessMessageManagerCallback); > } > > - virtual bool DoSendBlockingMessage(JSContext* aCx, > + bool DoSendBlockingMessage(JSContext* aCx, indentation ::: dom/base/nsFrameMessageManager.cpp:2130 (Diff revision 5) > true, &aData, &cpows, aPrincipal, aRetVal); > } > return true; > } > > - virtual nsresult DoSendAsyncMessage(JSContext* aCx, > + nsresult DoSendAsyncMessage(JSContext* aCx, indentation ::: dom/base/nsGlobalWindow.cpp:722 (Diff revision 5) > class nsOuterWindowProxy : public js::Wrapper > { > public: > constexpr nsOuterWindowProxy() : js::Wrapper(0) { } > > - virtual bool finalizeInBackground(const JS::Value& priv) const override { > + bool finalizeInBackground(const JS::Value& priv) const override { { in a new line ::: dom/base/nsGlobalWindow.cpp:727 (Diff revision 5) > - virtual bool finalizeInBackground(const JS::Value& priv) const override { > + bool finalizeInBackground(const JS::Value& priv) const override { > return false; > } > > // Standard internal methods > - virtual bool getOwnPropertyDescriptor(JSContext* cx, > + bool getOwnPropertyDescriptor(JSContext* cx, indentation... here and _everywhere_ in this patch :) ::: dom/base/nsGlobalWindow.cpp:9065 (Diff revision 5) > > // Try to match compartments that are not web content by matching compartments > // with principals that are either the system principal or an expanded principal. > // This may not return true for all non-web-content compartments. > struct BrowserCompartmentMatcher : public js::CompartmentFilter { > - virtual bool match(JSCompartment* c) const override > + bool match(JSCompartment* c) const override in the meantime: c => aC ::: dom/indexedDB/ActorsParent.cpp:5475 (Diff revision 5) > - virtual > - ~ConnectionRunnable() = default; > + > + ~ConnectionRunnable() override = default; remove these extra spaces ::: dom/indexedDB/ActorsParent.cpp:5900 (Diff revision 5) > , mActorDestroyed(false) > { > AssertIsOnOwningThread(); > } > > - virtual > + extra spaces ::: dom/indexedDB/ActorsParent.cpp:6106 (Diff revision 5) > TransactionDatabaseOperationBase(TransactionBase* aTransaction); > > TransactionDatabaseOperationBase(TransactionBase* aTransaction, > uint64_t aLoggingSerialNumber); > > - virtual > + ditto ::: dom/indexedDB/ActorsParent.cpp:7376 (Diff revision 5) > FactoryOp(Factory* aFactory, > already_AddRefed<ContentParent> aContentParent, > const CommonFactoryRequestParams& aCommonParams, > bool aDeleting); > > - virtual > + ditto ::: dom/indexedDB/ActorsParent.cpp:7771 (Diff revision 5) > } > > protected: > DatabaseOp(Database* aDatabase); > > - virtual > + ditto ::: dom/indexedDB/ActorsParent.cpp:7833 (Diff revision 5) > protected: > explicit VersionChangeTransactionOp(VersionChangeTransaction* aTransaction) > : TransactionDatabaseOperationBase(aTransaction) > { } > > - virtual > + ditto ::: dom/indexedDB/ActorsParent.cpp:8133 (Diff revision 5) > #ifdef DEBUG > , mResponseSent(false) > #endif > { } > > - virtual > + in this patch, there are many!
Attachment #8810354 - Flags: review?(amarchesini) → review+
Pushed by bpostelnicu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/494c49290b53 Converts for(...; ...; ...) loops to use the new range-based loops in C++11 in dom/. r=baku https://hg.mozilla.org/integration/autoland/rev/5b3a57978bd4 Use auto type specifier where aplicable for variable declarations to improve code readability and maintainability in dom/. r=baku https://hg.mozilla.org/integration/autoland/rev/c1c955254a8c Replace default bodies of special member functions with = default in dom/. r=baku https://hg.mozilla.org/integration/autoland/rev/de9b1fc4d31c Replace string literals containing escaped characters with raw string literals in dom/. r=baku https://hg.mozilla.org/integration/autoland/rev/06b5e559e2a8 Replace integer literals which are cast to bool in dom/. r=baku https://hg.mozilla.org/integration/autoland/rev/ad9ca936fa4a Use C++11's override and remove virtual where applicable in dom/ r=baku
Product: Core → Firefox Build System
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: