Closed Bug 1629853 Opened 5 years ago Closed 5 years ago

[Automated review] wrong clang-format

Categories

(Developer Infrastructure :: Source Code Analysis, defect)

defect

Tracking

(firefox77 fixed)

RESOLVED FIXED
mozilla77
Tracking Status
firefox77 --- fixed

People

(Reporter: sg, Assigned: andi)

References

Details

Attachments

(1 file)

Phabricator URL: https://phabricator.services.mozilla.com/D70383

clang-format wants to patch this:

--- xpcom/ds/nsTArray.h	2020-04-14 10:55:58.493946592 +0000
+++ xpcom/ds/nsTArray.h	2020-04-14 10:57:18.121946592 +0000
@@ -903,14 +903,14 @@
 template <class E, class Derived, typename Alloc>
 struct nsTArray_TypedBase<JS::Heap<E>, Derived, Alloc>
     : public nsTArray_SafeElementAtHelper<JS::Heap<E>, Derived, Alloc> {
-  operator const nsTArray<E>&() {
+  operator const nsTArray<E> &() {
     static_assert(sizeof(E) == sizeof(JS::Heap<E>),
                   "JS::Heap<E> must be binary compatible with E.");
     Derived* self = static_cast<Derived*>(this);
     return *reinterpret_cast<nsTArray<E>*>(self);
   }
 
-  operator const FallibleTArray<E>&() {
+  operator const FallibleTArray<E> &() {
     Derived* self = static_cast<Derived*>(this);
     return *reinterpret_cast<FallibleTArray<E>*>(self);
   }
@@ -1112,14 +1112,14 @@
   // Allow converting to a const array with a different kind of allocator,
   // Since the allocator doesn't matter for const arrays
   template <typename Allocator>
-  operator const nsTArray_Impl<E, Allocator>&() const& {
+  operator const nsTArray_Impl<E, Allocator> &() const& {
     return *reinterpret_cast<const nsTArray_Impl<E, Allocator>*>(this);
   }
   // And we have to do this for our subclasses too
-  operator const nsTArray<E>&() const& {
+  operator const nsTArray<E> &() const& {
     return *reinterpret_cast<const nsTArray<E>*>(this);
   }
-  operator const FallibleTArray<E>&() const& {
+  operator const FallibleTArray<E> &() const& {
     return *reinterpret_cast<const FallibleTArray<E>*>(this);
   }
 

This looks wrong to me, and my local clang-format rather seems right.

I already reported this bug:
https://bugs.llvm.org/show_bug.cgi?id=45357
caused by https://reviews.llvm.org/D69573
maybe we should backout the upstream change D69573

After I updated my local toolchain using mach bootstrap, I now get these strange formatting whenever I commit something in a file that contains a user-defined conversion operator returning a reference, which is pretty annoying. So can we provide a toolchain update that backs this change out?

Assignee: nobody → bpostelnicu
Status: NEW → ASSIGNED
Pushed by bpostelnicu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/69cb6fb25d2f revert D69573 from clang-format. r=sylvestre
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla77

I believe this was fixed by https://reviews.llvm.org/D76850 (which may have been post the v10 branch of LLVM)

The review shows this is only in the master branch of LLVM.

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: