dbus headers on Ubuntu Precise blocking C++11 (was Re: [chromium-dev] Using C++ lambdas and base::Callback)

60 views
Skip to first unread message

Paweł Hajdan, Jr.

unread,
Sep 4, 2013, 2:26:11 PM9/4/13
to Nico Weber, roc...@chromium.org, Chromium-dev, Vincent Scheib, Victor Khimenko, David Benjamin, Ury Zhilinsky
Hey, Nico, the dbus issue should be fixable.

First, gcc-4.6 and gcc-4.8 seem to do just fine with this if I understand correctly. If only gcc-4.7 is affected, it has ABI issues anyway and I think it's perfectly fine not to support it. Note that with Ubuntu Precise on 4.6 and Saucy on 4.8 we nicely jump over the problematic 4.7 version.

Another option is to try to get Ubuntu to backport http://cgit.freedesktop.org/dbus/dbus/commit/dbus/dbus-protocol.h?id=51b88b4c7919487290c0862b013cd8e7cd2de34b to Precise. There are chances for that.

I'm really wondering why gcc-4.7 is such a concern if it has other major problems anyway.

I'd be happy to help with Linux-related issues here.

For reference, #chromium irc logs from http://echelog.com/logs/browse/chromium/1375221600 :

[21:23:48] <phajdan-jr> thakis: can you see any clang-side workaround for this?
[21:23:51] <thakis> we could do the libspeech hack
[21:23:54] <thakis> and include our own header
[21:23:59] <thakis> phajdan-jr: the problem isn't clang
[21:24:00] <eglaysher> Give up and use a bundled version.
[21:24:02] <thakis> phajdan-jr: clang is happy
[21:24:13] <phajdan-jr> possible, yeah; or rewrite the system headers on the fly
[21:24:15] <thakis> phajdan-jr: this is a hard error in gcc 4.7 if we turn on c++11
[21:24:36] <thakis> "now run `sudo build-chromium` to build chromium. we won't do anything _weird_ to your system, promise"
[21:24:48] <phajdan-jr> thakis: is gcc 4.7 a problem for us? does 4.8 work?
[21:25:01] <thakis> it's a disablable warning in gcc 4.8
[21:25:27] <phajdan-jr> thakis: I meant no changes to actual header files on the system, it'd work on copies and do some trick similar to shim_headers.gypi for example
[21:25:40] <thakis> since gcc 4.7.0 and 4.7.1 have abi issues, not being to build with 4.7 might be a feature i suppose :-P
[21:25:57] <phajdan-jr> I'm currently looking whether we can just drop gcc 4.7... should be possible
[21:26:24] <thakis> sure, we can officially not support it, but people will try building with it anyhow
[21:26:34] <qengho> phajdan-jr: I might be able to help, if you discover that a stable-release update is best.
[21:26:34] <thakis> i'll go with the libspeechd route
[21:26:35] <phajdan-jr> thakis: alright, Precise has 4.6, Saucy has 4.8. SGTM
[21:28:26] <phajdan-jr> qengho: thanks - I'll contact you if we decide to go that way (unlikely, but still); how do you estimate the chances btw?
[21:28:35] <phajdan-jr> IMHO backporting http://cgit.freedesktop.org/dbus/dbus/commit/dbus/dbus-protocol.h?id=51b88b4c7919487290c0862b013cd8e7cd2de34b does not qualify for SRU
[21:28:54] <phajdan-jr> thakis: I'd rather drop gcc 4.7, seriously; bundling headers is bad
[21:29:16] <thakis> eh
[21:29:42] <phajdan-jr> seriously, is there any reason not to do that? :)
[21:30:01] <qengho> phajdan-jr: if I can point to an error in packaging, pretty good.
[21:30:47] <qengho> even if it's a code error upstream, it's not bad chances.
[21:31:28] <qengho> This is source, not a running program that could introduce all kinds of unintended effects.
[21:31:33] <phajdan-jr> thakis: would you like to give this a try? I think this would be the cleanest option. If this doesn't work or takes too long, my second choice would be to stop worrying about gcc 4.7.
[21:31:55] <thakis> "this" being?
[21:31:58] <phajdan-jr> I mean I would file an Ubuntu bug etc; just a question about a decision
[21:32:09] <phajdan-jr> thakis: this = ask Ubuntu to backport the dbus fix
[21:32:17] <thakis> nah, i'll just bundle the header
[21:32:46] <phajdan-jr> thakis: dropping gcc 4.7 would be cleaner; bundling headers is a mess
[21:33:05] <phajdan-jr> thakis: is there any reason to avoid the route I'm recommending?
[21:33:16] <thakis> yes, it's _more_ messy
Paweł

On Wed, Sep 4, 2013 at 10:30 AM, Nico Weber <tha...@chromium.org> wrote:
On Wed, Sep 4, 2013 at 10:19 AM, <roc...@chromium.org> wrote:
Are roadblocks for language features or toolchains being tracked? It would be great to see metabugs like "Allow {nullptr, auto, for-each loops} in Chromium source", branching out to more specific toolchain issues that could be tackled independently by interested developers.

There's really just one issue, "turn on c++11". See the thread https://groups.google.com/a/chromium.org/forum/#!searchin/chromium-dev/c$2B$2B11/chromium-dev/oVTPsijC7d0/3hs88g_9V2oJ for things you can to do help. Ubuntu Precise ships with code that isn't valid C++11 in the dbus headers for example.
 


On Wednesday, September 4, 2013 10:05:34 AM UTC-7, Albert J. Wong wrote:
Interesting trick.

Unfortunately, the limiting factor right now is still the lack of c++11 support in all toolchains. This is preventing us from using some other constructs that people really want (eg., nullptr, auto, for-each loops).

For lambdas, even if C++11 were fully supported, I'd be hesitant about throwing the switch 

The major thing to consider is how this interacts with Chromium smartpointers and and whether adding a second lambda-like construct (albeit the standard one) will actually help with readability.  However...until we have C++11 support, this discussion is largely theoretical.

-Albert


On Tue, Sep 3, 2013 at 10:37 AM, Vincent Scheib <sch...@chromium.org> wrote:
Ury has attempted to reply here, but the messages were deleted. So, here's a forward:

---------------------------------------------------------------------------------------
I was able to add a specialization to base::internal::RunnableAdapter to get the following syntax to work:
message_loop_->PostTask(FROM_HERE, base::Bind([this] () {
    // write some code
  }));

This also works with general purpose base::Callback-s and not just base::Clusure-s. What's interesting is that it also supports the good old functor objects and not just lambdas.

The trick is to use the type of the given lambda's operator() method to deduce the signature (search for decltype(&F::operator()) and also store the lambda itself inside of RunnableAdapter instead of a pointer to a function.

Here's a code snippet just in case that somebody is interested. Note that this is an add-on code which does not replace the existing code in any way.

namespace base { 
namespace internal {

template <typename F, typename Sig>
class RunnableAdapterFunctor;

template <typename F>
class RunnableAdapter
    : public RunnableAdapterFunctor<F, decltype(&F::operator())> {
 public:
  typedef RunnableAdapterFunctor<F, decltype(&F::operator())> BaseType;

  explicit RunnableAdapter(F f) : BaseType(f) {}
};

// Example for Arity 1
template <typename F, typename T, typename R, typename A1>
class RunnableAdapterFunctor<F, R (T::*)(A1) const>  {
 public:
  typedef R (RunType)(A1);

  RunnableAdapterFunctor(F f)
      : functor_(f) {
  }

  R Run(typename CallbackParamTraits<A1>::ForwardType a1) {
    return functor_(CallbackForward(a1));
  }

 private:
  F functor_;
};

} // namespace internal
} // namespace base


On Sat, Aug 31, 2013 at 6:38 AM, Victor Khimenko <kh...@chromium.org> wrote:


On Sat, Aug 31, 2013 at 4:14 PM, David Benjamin <davi...@chromium.org> wrote:

On Sat, Aug 31, 2013 at 4:05 AM, Victor Khimenko <kh...@chromium.org> wrote:


On Sat, Aug 31, 2013 at 4:07 AM, Ury Zhilinsky <uzhil...@google.com> wrote:
Hi,

Is it possible to use the two together somehow? Ideally, I'd like to be able to do something like:

  message_loop_->PostTask(FROM_HERE, [this] () {
    // write some code
  });

That's what everyone wishes to do, but unfortunately it's not something C++11 can offer. You can not convert lambda which grabs some value (including "this") to any type except "auto" which means you can not put in untemplateizied class which means you can not pass it around.

There are proposal for C++14 which will probably make it possible (see here: http://www.open-std.org/JTC1/SC22/WG21/docs/papers/2013/n3574.html), but nobody knows if it'll even be adopted, let alone implemented in all compilers and become available for Chromium codebase.

You should be able to wrap it in a std::function in C++11 though, right?

Sorry, my bad. You are right, of course.

Yeah, you can do that, but only if all modules are compiled as C++11. So it's just matter of waiting for the released of new toolchains, then.

--
--
Chromium Developers mailing list: chromi...@chromium.org

View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

--
--
Chromium Developers mailing list: chromi...@chromium.org

View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

Nico Weber

unread,
Sep 4, 2013, 2:29:43 PM9/4/13
to Paweł Hajdan, Jr., roc...@chromium.org, Chromium-dev, Vincent Scheib, Victor Khimenko, David Benjamin, Ury Zhilinsky
There's a bug for this, no need to discuss this on the mailing list: http://crbug.com/263960

On Wed, Sep 4, 2013 at 11:26 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:
Hey, Nico, the dbus issue should be fixable.

First, gcc-4.6 and gcc-4.8 seem to do just fine with this if I understand correctly. If only gcc-4.7 is affected, it has ABI issues anyway and I think it's perfectly fine not to support it.

We use gcc-4.7 for cros.
 
Note that with Ubuntu Precise on 4.6 and Saucy on 4.8 we nicely jump over the problematic 4.7 version.

Another option is to try to get Ubuntu to backport http://cgit.freedesktop.org/dbus/dbus/commit/dbus/dbus-protocol.h?id=51b88b4c7919487290c0862b013cd8e7cd2de34b to Precise. There are chances for that.

I'm really wondering why gcc-4.7 is such a concern if it has other major problems anyway.

I'd be happy to help with Linux-related issues here.

This is certainly fixable (as I say on IRC below, I think a better fix is to bundle a dbus header with a fix), it just needs to be done.

(There were other concerns on the C++11 thread, and there seemed a general lack of enthusiasm, so I figured we had decided to just wait.)

Paweł Hajdan, Jr.

unread,
Sep 5, 2013, 2:39:38 PM9/5/13
to Nico Weber, roc...@chromium.org, Chromium-dev, Vincent Scheib, Victor Khimenko, David Benjamin, Ury Zhilinsky
On Wed, Sep 4, 2013 at 11:29 AM, Nico Weber <tha...@chromium.org> wrote:
There's a bug for this, no need to discuss this on the mailing list: http://crbug.com/263960

On Wed, Sep 4, 2013 at 11:26 AM, Paweł Hajdan, Jr. <phajd...@chromium.org> wrote:
Hey, Nico, the dbus issue should be fixable.

First, gcc-4.6 and gcc-4.8 seem to do just fine with this if I understand correctly. If only gcc-4.7 is affected, it has ABI issues anyway and I think it's perfectly fine not to support it.

We use gcc-4.7 for cros.

It's easy to patch dbus under cros. I could do that.
 
Note that with Ubuntu Precise on 4.6 and Saucy on 4.8 we nicely jump over the problematic 4.7 version.

Another option is to try to get Ubuntu to backport http://cgit.freedesktop.org/dbus/dbus/commit/dbus/dbus-protocol.h?id=51b88b4c7919487290c0862b013cd8e7cd2de34b to Precise. There are chances for that.

I'm really wondering why gcc-4.7 is such a concern if it has other major problems anyway.

I'd be happy to help with Linux-related issues here.

This is certainly fixable (as I say on IRC below, I think a better fix is to bundle a dbus header with a fix), it just needs to be done.

Agreed it's fixable - bundling a header introduces a long-term maintainability problem though (at some point system dbus will be more recent, there actually might be mismatch between header and library, and all related problems).

What if I just fixed dbus on cros? Is that the only remaining thing for this issue? How would I test it in ChromeOS to make sure it works?
 
(There were other concerns on the C++11 thread, and there seemed a general lack of enthusiasm, so I figured we had decided to just wait.)

Right - maybe just waiting for gcc-4.8 is the answer then? Or switching to clang?

I'd just really like to avoid bundling more headers if possible.

Paweł
Reply all
Reply to author
Forward
0 new messages