For this code and other instances like it in PrefsHelper.h: const auto& jstrVal = type == widget::PrefsHelper::PREF_STRING ? jni::StringParam(strVal, aPrefName.Env()) : jni::StringParam(nullptr); clang says: /home/froydnj/src/gecko-dev.git/widget/android/PrefsHelper.h:71:17: error: call to implicitly-deleted copy constructor of 'jni::StringParam' jni::StringParam(strVal, aPrefName.Env()) : ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ /opt/build/froydnj/build-android/dist/include/mozilla/jni/Refs.h:635:21: note: copy constructor of 'StringParam' is implicitly deleted because base class 'String::Ref' (aka 'Ref<mozilla::jni::TypedObject<jstring>, _jstring *>') has an inaccessible copy constructor class StringParam : public String::Ref ^ From reading through the code and comments, a deleted copy constructor looks like an explicit design choice. clang is following the letter of the spec here; I think GCC is either getting name lookup completely wrong, or doing something like: char data[sizeof(StringParam)]; if (type == TYPE_STRING) { new (&data) StringParam(...); } else { new (&data) StringParam(nullptr); } const auto& jstrVal = reinterpret_cast<...>(...); which would be a little tricky, but totally doable. I have tried to rewrite things in such a way as to ensure that we don't do any extra work for the non-TYPE_STRING case, but I am stumped. The best idea I have come up with involves mozilla::Maybe: Maybe<const StringParam> jstrVal; jstrVal.emplace(nullptr); if (type == TYPE_STRING) { jstrVal.reset(); jstrVal.emplace(...); } but that is ugly, although it takes care of any necessary destructors for us. Do you have any better ideas?

In an expression such as: const auto& x = cond() ? AClass(...) : AClass(); the C++ standard specifies that the copy constructor of AClass is invoked on the result of the conditional expression ([expr.cond]p6). GCC does not honor this part of the specification, whereas clang does; clang therefore complains about instances of code such as: const auto& jstrVal = type == widget::PrefsHelper::PREF_STRING ? jni::StringParam(strVal, aPrefName.Env()) : jni::StringParam(nullptr); as jni::StringParam is not copy-constructable. The simplest solution that does not introduce unnecessary allocation uses mozilla::Maybe to hold the temporary objects and to hide some of the details of constructing objects in-place. The compiler may even be able to optimize away some of the unnnecessary checks that Maybe introduces (e.g. checking for whether the Maybe is a Some or None at certain points).

Comment on attachment 8761258 [details] [diff] [review] avoid invalid copying of objects in PrefsHelper.h Review of attachment 8761258 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!

Actually, I wonder if we can add a move constructor to StringParam. Design-wise, I think it should be fine to have a move constructor in StringParam.

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/d4f903cba94d avoid invalid copying of objects in PrefsHelper.h; r=darchons