Skip to content
This repository has been archived by the owner on Mar 26, 2020. It is now read-only.

Generating constexpr for constants when possible #282

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Generating constexpr for constants when possible #282

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 45 additions & 9 deletions src/source/CppGenerator.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,34 @@ class CppGenerator(spec: Spec) extends Generator(spec) {
}

def generateHppConstants(w: IndentWriter, consts: Seq[Const]) = {
var needsCppGen = false

for (c <- consts) {
var isConstExpr = true
val constValue = c.value match {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be clearer to match on c.ty instead. I'm not set on that, since this does make the code simpler, but it took me a while to figure out that this distinction is the reason that optionals are being treated as constexpr-eligible. It's not an explicit case statement anywhere, but simply implicit in the fact that the constant value for an optional is of type T.

case l: Long => " = { " + l.toString + " };"
case d: Double if marshal.fieldType(c.ty) == "float" => " = { " + d.toString + "f };"
case d: Double => " = { " + d.toString + " };"
case b: Boolean => if (b) " = { true };" else " = { false };"
case e: EnumValue => " = " + marshal.typename(c.ty) + "::" + idCpp.enum(e.ty.name + "_" + e.name) + ";"
case _ => {
needsCppGen = true;
isConstExpr = false;
";"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest making constValue empty here, and applying the semi-colon in the final statement below. The semicolon isn't really part of the const value.

}
}

w.wl
writeDoc(w, c.doc)
w.wl(s"static ${marshal.fieldType(c.ty)} const ${idCpp.const(c.ident)};")
w.w(s"static ${if (isConstExpr) "constexpr" else "" } const ${marshal.fieldType(c.ty)} ${idCpp.const(c.ident)}${constValue}")
}

if (!consts.isEmpty) {
w.wl
w.wl
}

needsCppGen
}

def generateCppConstants(w: IndentWriter, consts: Seq[Const], selfName: String) = {
Expand Down Expand Up @@ -127,10 +150,20 @@ class CppGenerator(spec: Spec) extends Generator(spec) {

val skipFirst = SkipFirst()
for (c <- consts) {
skipFirst{ w.wl }
w.w(s"${marshal.fieldType(c.ty)} const $selfName::${idCpp.const(c.ident)} = ")
writeCppConst(w, c.ty, c.value)
w.wl(";")
val isConstExpr = c.value match {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to unify the cases for this match with the ones above in the header file. One option would be to make a helper function which returns the constValue used in the header file, then have isConstExpr here simply test whether that value is non-empty.

case l: Long => true
case d: Double => true
case b: Boolean => true
case e: EnumValue => true
case _ => false
}

if (!isConstExpr) {
skipFirst{ w.wl }
w.w(s"${marshal.fieldType(c.ty)} const $selfName::${idCpp.const(c.ident)} = ")
writeCppConst(w, c.ty, c.value)
w.wl(";")
}
}
}

Expand All @@ -149,6 +182,8 @@ class CppGenerator(spec: Spec) extends Generator(spec) {
refs.cpp.add("#include "+q(spec.cppExtendedRecordIncludePrefix + spec.cppFileIdentStyle(ident) + "." + spec.cppHeaderExt))
}

var constantsRequireCpp = false;

// C++ Header
def writeCppPrototype(w: IndentWriter) {
if (r.ext.cpp) {
Expand All @@ -159,7 +194,7 @@ class CppGenerator(spec: Spec) extends Generator(spec) {
writeDoc(w, doc)
writeCppTypeParams(w, params)
w.w("struct " + actualSelf + cppFinal).bracedSemi {
generateHppConstants(w, r.consts)
constantsRequireCpp = generateHppConstants(w, r.consts)
// Field definitions.
for (f <- r.fields) {
writeDoc(w, f.doc)
Expand Down Expand Up @@ -210,7 +245,7 @@ class CppGenerator(spec: Spec) extends Generator(spec) {

writeHppFile(cppName, origin, refs.hpp, refs.hppFwds, writeCppPrototype)

if (r.consts.nonEmpty || r.derivingTypes.nonEmpty) {
if (constantsRequireCpp || r.derivingTypes.nonEmpty) {
writeCppFile(cppName, origin, refs.cpp, w => {
generateCppConstants(w, r.consts, actualSelf)

Expand Down Expand Up @@ -274,6 +309,7 @@ class CppGenerator(spec: Spec) extends Generator(spec) {

val self = marshal.typename(ident, i)
val methodNamesInScope = i.methods.map(m => idCpp.method(m.ident))
var generateCppFile = false;

writeHppFile(ident, origin, refs.hpp, refs.hppFwds, w => {
writeDoc(w, doc)
Expand All @@ -283,7 +319,7 @@ class CppGenerator(spec: Spec) extends Generator(spec) {
// Destructor
w.wl(s"virtual ~$self() {}")
// Constants
generateHppConstants(w, i.consts)
generateCppFile = generateHppConstants(w, i.consts)
// Methods
for (m <- i.methods) {
w.wl
Expand All @@ -301,7 +337,7 @@ class CppGenerator(spec: Spec) extends Generator(spec) {
})

// Cpp only generated in need of Constants
if (i.consts.nonEmpty) {
if (generateCppFile) {
writeCppFile(ident, origin, refs.cpp, w => {
generateCppConstants(w, i.consts, self)
})
Expand Down
30 changes: 0 additions & 30 deletions test-suite/generated-src/cpp/constants.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,34 +5,6 @@

namespace testsuite {

bool const Constants::BOOL_CONSTANT = true;

int8_t const Constants::I8_CONSTANT = 1;

int16_t const Constants::I16_CONSTANT = 2;

int32_t const Constants::I32_CONSTANT = 3;

int64_t const Constants::I64_CONSTANT = 4;

float const Constants::F32_CONSTANT = 5.0f;

double const Constants::F64_CONSTANT = 5.0;

std::experimental::optional<bool> const Constants::OPT_BOOL_CONSTANT = true;

std::experimental::optional<int8_t> const Constants::OPT_I8_CONSTANT = 1;

std::experimental::optional<int16_t> const Constants::OPT_I16_CONSTANT = 2;

std::experimental::optional<int32_t> const Constants::OPT_I32_CONSTANT = 3;

std::experimental::optional<int64_t> const Constants::OPT_I64_CONSTANT = 4;

std::experimental::optional<float> const Constants::OPT_F32_CONSTANT = 5.0;

std::experimental::optional<double> const Constants::OPT_F64_CONSTANT = 5.0;

std::string const Constants::STRING_CONSTANT = {"string-constant"};

std::experimental::optional<std::string> const Constants::OPT_STRING_CONSTANT = {"string-constant"};
Expand All @@ -41,6 +13,4 @@ ConstantRecord const Constants::OBJECT_CONSTANT = ConstantRecord(
Constants::I32_CONSTANT /* some_integer */ ,
Constants::STRING_CONSTANT /* some_string */ );

bool const Constants::DUMMY = false;

} // namespace testsuite
54 changes: 19 additions & 35 deletions test-suite/generated-src/cpp/constants.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,57 +15,41 @@ namespace testsuite {
struct Constants final {

/** bool_constant has documentation. */
static bool const BOOL_CONSTANT;

static int8_t const I8_CONSTANT;

static int16_t const I16_CONSTANT;

static int32_t const I32_CONSTANT;

static int64_t const I64_CONSTANT;

static float const F32_CONSTANT;

static constexpr const bool BOOL_CONSTANT = { true };
static constexpr const int8_t I8_CONSTANT = { 1 };
static constexpr const int16_t I16_CONSTANT = { 2 };
static constexpr const int32_t I32_CONSTANT = { 3 };
static constexpr const int64_t I64_CONSTANT = { 4 };
static constexpr const float F32_CONSTANT = { 5.0f };
/**
* f64_constant has long documentation.
* (Second line of multi-line documentation.
* Indented third line of multi-line documentation.)
*/
static double const F64_CONSTANT;

static std::experimental::optional<bool> const OPT_BOOL_CONSTANT;

static std::experimental::optional<int8_t> const OPT_I8_CONSTANT;

static constexpr const double F64_CONSTANT = { 5.0 };
static constexpr const std::experimental::optional<bool> OPT_BOOL_CONSTANT = { true };
static constexpr const std::experimental::optional<int8_t> OPT_I8_CONSTANT = { 1 };
/** opt_i16_constant has documentation. */
static std::experimental::optional<int16_t> const OPT_I16_CONSTANT;

static std::experimental::optional<int32_t> const OPT_I32_CONSTANT;

static std::experimental::optional<int64_t> const OPT_I64_CONSTANT;

static constexpr const std::experimental::optional<int16_t> OPT_I16_CONSTANT = { 2 };
static constexpr const std::experimental::optional<int32_t> OPT_I32_CONSTANT = { 3 };
static constexpr const std::experimental::optional<int64_t> OPT_I64_CONSTANT = { 4 };
/**
* opt_f32_constant has long documentation.
* (Second line of multi-line documentation.
* Indented third line of multi-line documentation.)
*/
static std::experimental::optional<float> const OPT_F32_CONSTANT;

static std::experimental::optional<double> const OPT_F64_CONSTANT;

static std::string const STRING_CONSTANT;

static std::experimental::optional<std::string> const OPT_STRING_CONSTANT;

static ConstantRecord const OBJECT_CONSTANT;

static constexpr const std::experimental::optional<float> OPT_F32_CONSTANT = { 5.0 };
static constexpr const std::experimental::optional<double> OPT_F64_CONSTANT = { 5.0 };
static const std::string STRING_CONSTANT;
static const std::experimental::optional<std::string> OPT_STRING_CONSTANT;
static const ConstantRecord OBJECT_CONSTANT;
/**
* No support for null optional constants
* No support for optional constant records
* No support for constant binary, list, set, map
*/
static bool const DUMMY;
static constexpr const bool DUMMY = { false };

};

} // namespace testsuite
28 changes: 0 additions & 28 deletions test-suite/generated-src/cpp/constants_interface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,34 +6,6 @@

namespace testsuite {

bool const ConstantsInterface::BOOL_CONSTANT = true;

int8_t const ConstantsInterface::I8_CONSTANT = 1;

int16_t const ConstantsInterface::I16_CONSTANT = 2;

int32_t const ConstantsInterface::I32_CONSTANT = 3;

int64_t const ConstantsInterface::I64_CONSTANT = 4;

float const ConstantsInterface::F32_CONSTANT = 5.0f;

double const ConstantsInterface::F64_CONSTANT = 5.0;

std::experimental::optional<bool> const ConstantsInterface::OPT_BOOL_CONSTANT = true;

std::experimental::optional<int8_t> const ConstantsInterface::OPT_I8_CONSTANT = 1;

std::experimental::optional<int16_t> const ConstantsInterface::OPT_I16_CONSTANT = 2;

std::experimental::optional<int32_t> const ConstantsInterface::OPT_I32_CONSTANT = 3;

std::experimental::optional<int64_t> const ConstantsInterface::OPT_I64_CONSTANT = 4;

std::experimental::optional<float> const ConstantsInterface::OPT_F32_CONSTANT = 5.0;

std::experimental::optional<double> const ConstantsInterface::OPT_F64_CONSTANT = 5.0;

std::string const ConstantsInterface::STRING_CONSTANT = {"string-constant"};

std::experimental::optional<std::string> const ConstantsInterface::OPT_STRING_CONSTANT = {"string-constant"};
Expand Down
49 changes: 17 additions & 32 deletions test-suite/generated-src/cpp/constants_interface.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,51 +16,36 @@ class ConstantsInterface {
public:
virtual ~ConstantsInterface() {}

static bool const BOOL_CONSTANT;

static int8_t const I8_CONSTANT;

static int16_t const I16_CONSTANT;

static constexpr const bool BOOL_CONSTANT = { true };
static constexpr const int8_t I8_CONSTANT = { 1 };
static constexpr const int16_t I16_CONSTANT = { 2 };
/** i32_constant has documentation. */
static int32_t const I32_CONSTANT;

static constexpr const int32_t I32_CONSTANT = { 3 };
/**
* i64_constant has long documentation.
* (Second line of multi-line documentation.
* Indented third line of multi-line documentation.)
*/
static int64_t const I64_CONSTANT;

static float const F32_CONSTANT;

static double const F64_CONSTANT;

static std::experimental::optional<bool> const OPT_BOOL_CONSTANT;

static std::experimental::optional<int8_t> const OPT_I8_CONSTANT;

static constexpr const int64_t I64_CONSTANT = { 4 };
static constexpr const float F32_CONSTANT = { 5.0f };
static constexpr const double F64_CONSTANT = { 5.0 };
static constexpr const std::experimental::optional<bool> OPT_BOOL_CONSTANT = { true };
static constexpr const std::experimental::optional<int8_t> OPT_I8_CONSTANT = { 1 };
/** opt_i16_constant has documentation. */
static std::experimental::optional<int16_t> const OPT_I16_CONSTANT;

static std::experimental::optional<int32_t> const OPT_I32_CONSTANT;

static std::experimental::optional<int64_t> const OPT_I64_CONSTANT;

static constexpr const std::experimental::optional<int16_t> OPT_I16_CONSTANT = { 2 };
static constexpr const std::experimental::optional<int32_t> OPT_I32_CONSTANT = { 3 };
static constexpr const std::experimental::optional<int64_t> OPT_I64_CONSTANT = { 4 };
/**
* opt_f32_constant has long documentation.
* (Second line of multi-line documentation.
* Indented third line of multi-line documentation.)
*/
static std::experimental::optional<float> const OPT_F32_CONSTANT;

static std::experimental::optional<double> const OPT_F64_CONSTANT;

static std::string const STRING_CONSTANT;

static std::experimental::optional<std::string> const OPT_STRING_CONSTANT;
static constexpr const std::experimental::optional<float> OPT_F32_CONSTANT = { 5.0 };
static constexpr const std::experimental::optional<double> OPT_F64_CONSTANT = { 5.0 };
static const std::string STRING_CONSTANT;
static const std::experimental::optional<std::string> OPT_STRING_CONSTANT;
static const ConstantRecord OBJECT_CONSTANT;

static ConstantRecord const OBJECT_CONSTANT;

/**
* No support for null optional constants
Expand Down
3 changes: 2 additions & 1 deletion test-suite/generated-src/cpp/extended_record_base.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ struct ExtendedRecord; // Requiring extended class
/** Extended record */
struct ExtendedRecordBase {

static ExtendedRecord const EXTENDED_RECORD_CONST;
static const ExtendedRecord EXTENDED_RECORD_CONST;

bool foo;

ExtendedRecordBase(bool foo_)
Expand Down
3 changes: 2 additions & 1 deletion test-suite/generated-src/cpp/interface_using_extended_record.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ class InterfaceUsingExtendedRecord {
public:
virtual ~InterfaceUsingExtendedRecord() {}

static RecordUsingExtendedRecord const CR;
static const RecordUsingExtendedRecord CR;


virtual ExtendedRecord meth(const ExtendedRecord & er) = 0;
};
Expand Down
3 changes: 2 additions & 1 deletion test-suite/generated-src/cpp/record_using_extended_record.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ namespace testsuite {

struct RecordUsingExtendedRecord final {

static RecordUsingExtendedRecord const CR;
static const RecordUsingExtendedRecord CR;

ExtendedRecord er;

RecordUsingExtendedRecord(ExtendedRecord er_)
Expand Down