Skip to content

Commit

Permalink
Fix portability issues with long double in modcc. (#1246)
Browse files Browse the repository at this point in the history
Change from long double to double for storing floating point values in modcc.

WSL, and Windows generally, treats long double differently than Linux, which leads to inconsistencies in literal numeric values in code generated by modcc on the two platforms.

Fixes #1245
  • Loading branch information
brenthuisman authored Nov 19, 2020
1 parent 9233b78 commit d75a1bc
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 18 deletions.
8 changes: 4 additions & 4 deletions modcc/expression.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -330,11 +330,11 @@ class NumberExpression : public Expression {
: Expression(loc), value_(std::stold(value))
{}

NumberExpression(Location loc, long double value)
NumberExpression(Location loc, double value)
: Expression(loc), value_(value)
{}

virtual long double value() const {return value_;};
virtual double value() const {return value_;};

std::string to_string() const override {
return purple(pprintf("%", value_));
Expand All @@ -350,7 +350,7 @@ class NumberExpression : public Expression {

void accept(Visitor *v) override;
private:
long double value_;
double value_;
};

// an integral number
Expand All @@ -361,7 +361,7 @@ class IntegerExpression : public NumberExpression {
{}

IntegerExpression(Location loc, long long integer)
: NumberExpression(loc, static_cast<long double>(integer)), integer_(integer)
: NumberExpression(loc, static_cast<double>(integer)), integer_(integer)
{}

long long integer_value() const {return integer_;}
Expand Down
6 changes: 3 additions & 3 deletions modcc/symdiff.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ class SymPDiffVisitor: public Visitor, public error_stack {
std::string id_;
};

long double expr_value(Expression* e) {
double expr_value(Expression* e) {
return e && e->is_number()? e->is_number()->value(): NAN;
}

Expand All @@ -273,7 +273,7 @@ class ConstantSimplifyVisitor: public Visitor {
static bool is_number(Expression* e) { return e && e->is_number(); }
static bool is_number(const expression_ptr& e) { return is_number(e.get()); }

void as_number(Location loc, long double v) {
void as_number(Location loc, double v) {
result_ = make_expression<NumberExpression>(loc, v);
}

Expand All @@ -293,7 +293,7 @@ class ConstantSimplifyVisitor: public Visitor {
result_ = nullptr;
}

long double value() const { return expr_value(result_); }
double value() const { return expr_value(result_); }

bool is_number() const { return is_number(result_); }

Expand Down
6 changes: 3 additions & 3 deletions modcc/symdiff.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ bool involves_identifier(Expression* e, const identifier_set& ids);
expression_ptr constant_simplify(Expression* e);

// Extract value of expression that is a NumberExpression, or else return NAN.
long double expr_value(Expression* e);
double expr_value(Expression* e);

// Test if expression is a NumberExpression with value zero.
inline bool is_zero(Expression* e) {
Expand Down Expand Up @@ -57,11 +57,11 @@ inline expression_ptr constant_simplify(const expression_ptr& e) {
return constant_simplify(e.get());
}

inline long double expr_value(const expression_ptr& e) {
inline double expr_value(const expression_ptr& e) {
return expr_value(e.get());
}

inline long double is_zero(const expression_ptr& e) {
inline double is_zero(const expression_ptr& e) {
return is_zero(e.get());
}

Expand Down
10 changes: 2 additions & 8 deletions test/unit-modcc/test_parser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ TEST(Parser, parse_conserve) {
}
}

long double eval(Expression *e) {
double eval(Expression *e) {
if (auto n = e->is_number()) {
return n->value();
}
Expand All @@ -606,7 +606,7 @@ long double eval(Expression *e) {
default:;
}
}
return std::numeric_limits<long double>::quiet_NaN();
return std::numeric_limits<double>::quiet_NaN();
}

// test parsing of expressions for correctness
Expand Down Expand Up @@ -654,12 +654,6 @@ TEST(Parser, parse_binop) {
for (const auto& test_case: tests) {
std::unique_ptr<Expression> e;
EXPECT_TRUE(check_parse(e, &Parser::parse_expression, test_case.first));

// A loose tolerance of 1e-10 is required here because the eval()
// function uses long double for intermediate results (like constant
// folding in modparser). For expressions with transcendental
// operations this can see relatively large divergence between the
// double and long double results.
EXPECT_NEAR(eval(e.get()), test_case.second, 1e-10);
}
}
Expand Down

0 comments on commit d75a1bc

Please sign in to comment.