Bug 1278314 - avoid invalid copying of objects in PrefsHelper.h; r=darchons

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).
This commit is contained in:
Nathan Froyd 2016-06-20 18:37:13 -04:00
Родитель caa9d85a69
Коммит 9158ecd1de
1 изменённых файлов: 23 добавлений и 13 удалений

Просмотреть файл

@ -12,6 +12,7 @@
#include "nsCOMPtr.h"
#include "nsVariant.h"
#include "mozilla/Maybe.h"
#include "mozilla/Preferences.h"
#include "mozilla/Services.h"
#include "mozilla/UniquePtr.h"
@ -67,16 +68,19 @@ class PrefsHelper
return false;
}
const auto& jstrVal = type == widget::PrefsHelper::PREF_STRING ?
jni::StringParam(strVal, aPrefName.Env()) :
jni::StringParam(nullptr);
Maybe<jni::StringParam> jstrVal;
jstrVal.emplace(nullptr);
if (type == widget::PrefsHelper::PREF_STRING) {
jstrVal.reset();
jstrVal.emplace(strVal, aPrefName.Env());
}
if (aPrefHandler) {
widget::PrefsHelper::CallPrefHandler(
aPrefHandler, type, aPrefName, boolVal, intVal, jstrVal);
aPrefHandler, type, aPrefName, boolVal, intVal, jstrVal.ref());
} else {
widget::PrefsHelper::OnPrefChange(
aPrefName, type, boolVal, intVal, jstrVal);
aPrefName, type, boolVal, intVal, jstrVal.ref());
}
return true;
}
@ -189,12 +193,15 @@ public:
continue;
}
const auto& jstrVal = type == widget::PrefsHelper::PREF_STRING ?
jni::StringParam(strVal, aCls.Env()) :
jni::StringParam(nullptr);
Maybe<jni::StringParam> jstrVal;
jstrVal.emplace(nullptr);
if (type == widget::PrefsHelper::PREF_STRING) {
jstrVal.reset();
jstrVal.emplace(strVal, aCls.Env());
}
widget::PrefsHelper::CallPrefHandler(
aPrefHandler, type, nameStr, boolVal, intVal, jstrVal);
aPrefHandler, type, nameStr, boolVal, intVal, jstrVal.ref());
}
widget::PrefsHelper::CallPrefHandler(
@ -314,11 +321,14 @@ public:
return;
}
const auto& jstrVal = type == widget::PrefsHelper::PREF_STRING ?
jni::StringParam(strVal) :
jni::StringParam(nullptr);
Maybe<jni::StringParam> jstrVal;
jstrVal.emplace(nullptr);
if (type == widget::PrefsHelper::PREF_STRING) {
jstrVal.reset();
jstrVal.emplace(strVal);
}
widget::PrefsHelper::OnPrefChange(name, type, boolVal, intVal, jstrVal);
widget::PrefsHelper::OnPrefChange(name, type, boolVal, intVal, jstrVal.ref());
}
};