search

prost-planner / prost

probabilistic planning system for tasks encoded in RDDL
MIT License
41 stars 17 forks source link

compiler complaint in calculate_domain_as_interval.cc #131

Closed fdouglis closed 3 years ago

fdouglis commented 3 years ago

When I moved to a new system and rebuilt prost, I ran into a set of complaints in one file. One example:

[...]/src/rddl_parser/logical_expressions_includes/calculate_domain_as_interval.cc:189:19: error: suggest parentheses around assignment used as truth value [-Werror=parentheses] 189 | assert(minRes = numeric_limits::max());

At first I thought this was a typo and it was supposed to be == (and perhaps it is). I changed it to ==, and it built, but I hit that other error I mentioned with the new code for identifying those errors (#128). I tried changing it to what I think the original code did: assert((minRes = numeric_limits<double>::max()) != 0); but I hit the same bug.

I suspect this has nothing to do with #128 and simply an artifact of building on a very recent system (FC33), and it's a syntax issue you might want to address in the master repo at your convenience.

thomaskeller79 commented 3 years ago

Thanks, this is certainly a bug. The the true intention should be exactly as you propose (i.e., comparison rather than assignment).

fdouglis commented 3 years ago

Aha, thanks. And you are most welcome.

thomaskeller79 commented 3 years ago

I fixed this. @fdougliy, could you please pull and check if the compilation error is gone?

@geisserf: I accidentally violated our workflow in the development repo, and decided after that first violation not to go through the hassle of undoing everything but ignore the workflow altogether. For this reason, this issue is already merged.

fdouglis commented 3 years ago

Yup, builds without complaint now. And I see the real test was for the bounds, and not that the parameters were already set to the bounds. So the change I'd made wasn't really right either, though it didn't complain at runtime.

thomaskeller79 commented 3 years ago

Fixed in commit de09e63f34e06e38108979d3e02468cfa8ba230e