New issue
Advanced search Search tips

Issue 706963 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Android compiler can't implement some noexcept move constructors as "=default"

Project Member Reported by brettw@chromium.org, Mar 30 2017

Issue description

Ideally one should be able to define a class with a noexcept move constructor and implement it with "=default":

  Foo::Foo(Foo&&) noexcept = default;

This works everywhere except on our current Android toolchain which fails under specific cases. It fails when the class has members of certain STL containers that "should" be noexcept move constructible, but something gets lost.

For example, if the Foo class from above has a std::string and an int member, "noexcept = default" works. If it has a base::string16 or a std::vector, it will give an error unless you remove either "noexcept" or "= default" (and implement it manually).

This is a tracking bug so when we update the toolchain, we can know to fix code that references this bug.
 

Comment 1 by cmasso@google.com, Feb 14 2018

Components: Build Infra
Components: -Infra
This is not an infra issue, removing component:Infra.

Comment 3 by dcheng@chromium.org, Apr 10 2018

Cc: jbudorick@chromium.org dcheng@chromium.org
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 10 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62ede92bcf2a4ab69de40d55ef7d176771f6d597

commit 62ede92bcf2a4ab69de40d55ef7d176771f6d597
Author: Daniel Cheng <dcheng@chromium.org>
Date: Tue Apr 10 23:37:27 2018

Change various move constructors to be noexcept = default.

Declaring move constructors and assignment operators noexcept
allows STL containers to use moves instead of copies, which is
generally more efficient.

Previously, there were compiler issues that prevented this from
working [1]. They seem to have mysteriously fixed themselves
and testing on https://godbolt.org/g/TqfqtK seems to indicate that
the previously highlighted problematic test cases now work.

[1] https://groups.google.com/a/chromium.org/forum/#!topic/cxx/ze4WJFg7RvU

Bug:  706963 
Change-Id: Ieb73b4c53ab4235838265f1d968d2a57aa34a737
Reviewed-on: https://chromium-review.googlesource.com/1003872
Reviewed-by: John Budorick <jbudorick@chromium.org>
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Nico Weber <thakis@chromium.org>
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#549678}
[modify] https://crrev.com/62ede92bcf2a4ab69de40d55ef7d176771f6d597/components/history/core/browser/history_types.cc
[modify] https://crrev.com/62ede92bcf2a4ab69de40d55ef7d176771f6d597/components/history/core/browser/url_row.cc
[modify] https://crrev.com/62ede92bcf2a4ab69de40d55ef7d176771f6d597/components/query_parser/snippet.cc
[modify] https://crrev.com/62ede92bcf2a4ab69de40d55ef7d176771f6d597/content/public/common/presentation_connection_message.cc

Comment 5 by thakis@chromium.org, Apr 10 2018

(As mentioned on the review, nothing mysterious about it: we switched chrome/android to clang late last year)

Comment 6 by dcheng@chromium.org, Apr 21 2018

Owner: thakis@chromium.org
Status: Fixed (was: Untriaged)

Sign in to add a comment