1629853 - [Automated review] wrong clang-format

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Simon Giesecke [:sg] [he/him]]

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.

Comment 1

[Sylvestre Ledru [:Sylvestre]]

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

Comment 2

[Simon Giesecke [:sg] [he/him]]

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?

Comment 3

[Andi [:andi]]

Attachment: Bug 1629853 - revert D69573 from clang-format.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/69cb6fb25d2f
revert D69573 from clang-format. r=sylvestre

Comment 5

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/69cb6fb25d2f

Comment 6

[MyDeveloperDay]

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.