Adds a DRAKE_THROW_IF macro

[AlexandreAmice]

This change is 

[AlexandreAmice]

Sometimes I feel like it'd be more natural to express these types of check by saying THROW_IF instead of THROW_UNLESS. If there is a reason this macro was never added feel free to close this.

+@jwnimmer-tri for feature review. Thanks!

Reviewable status: LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, missing label for release notes (waiting on @AlexandreAmice)

[jwnimmer-tri]

Sometimes I feel like it'd be more natural to express these types of check by saying THROW_IF instead of THROW_UNLESS.

That's the right question to ponder, but this PR doesn't help do it yet. When adding new sugar (like in this PR), the PR should also convert 3+ call sites to use it, as evidence/demonstration of why it's useful. We can't have the project discuss this idea until we have examples in hand.

Reviewable status: 2 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, missing label for release notes (waiting on @AlexandreAmice)

---

common/drake_throw.h line 38 at r1 (raw file):

    typedef ::drake::assert::ConditionTraits<                                 \
        typename std::remove_cv_t<decltype(condition)>>                       \
        Trait;                                                                \

nit Unrelated whitespace change.

---

common/drake_throw.h line 52 at r1 (raw file):

/// Provides the same behavior as DRAKE_THROW_UNLESS, except that it throws an
/// exception iff the value is true.
#define DRAKE_THROW_IF(condition) DRAKE_THROW_UNLESS(!condition)

nit Imagine condition was foo && bar. This macro would expand to !foo && bar. We need parens to ensure a correct order of operations. (This is generally true for all macro arguments -- when writing a macro, typically all uses of arguments must be enclosed in parens.

Suggestion:

DRAKE_THROW_UNLESS(!(condition))

[AlexandreAmice]

I changed a few call sites that I thought might look nicer with THROW_IF rather than THROW_UNLESS.

Reviewable status: 3 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

---

geometry/proximity/volume_mesh_refiner.cc line 184 at r2 (raw file):

std::vector<int> VolumeMeshRefiner::GetTetrahedraOnTriangle(int v0, int v1,
                                                            int v2) const {
  DRAKE_THROW_IF(v0== v1 || v1 == v2 || v2 != v0);

An example where the boolean logic is a bit cleaner.

---

geometry/optimization/hpolyhedron.cc line 922 at r2 (raw file):

  MatrixXd circumbody_A = circumbody.A();
  VectorXd circumbody_b = circumbody.b();
  DRAKE_THROW_IF(circumbody.IsEmpty());

An example where it's more natural to say THROW_IF(empty) rather than THROW_UNLESS(not empty)

---

multibody/plant/multibody_plant.cc line 782 at r2 (raw file):

  }

  DRAKE_THROW_IF(*lower_limit == -kInf && *upper_limit == kInf);

Ditto. Cleaner boolean logic imo.

[jwnimmer-tri]

Reviewed 1 of 6 files at r2, 2 of 3 files at r3.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), needs at least two assigned reviewers, commits need curation (https://drake.mit.edu/reviewable.html#curated-commits), missing label for release notes (waiting on @AlexandreAmice)

---

geometry/optimization/hpolyhedron.cc line 922 at r2 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

An example where it's more natural to say THROW_IF(empty) rather than THROW_UNLESS(not empty)

For this one, I agree that it is epsilon better.

However, I'm not sure that the small benefit of eating the one ! character is worth the cost of developers needing to memorize a new macro, and the cost of beating back all of the inappropriate uses of the new macro during code review (as seen elsewhere in this PR).

Maybe you can try to find a few more appropriate uses, to paint a better picture of the upside?

---

geometry/proximity/volume_mesh_refiner.cc line 184 at r2 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

An example where the boolean logic is a bit cleaner.

Actually this one reads worse to me. To me, the prior code clearly says "require no duplicates". It also exactly matches the precondition in the API documentation. Having the checks have the same truth-sense as the documented preconditions is an important benefit of THROW_UNLESS vsTHROW_IF. The uses of THROW_IF are likely to be more when the API says "throws exception when ..." (throws when true) than preconditions (throws when false).

---

multibody/plant/multibody_plant.cc line 782 at r2 (raw file):

Previously, AlexandreAmice (Alexandre Amice) wrote…

Ditto. Cleaner boolean logic imo.

This change only looks better because the original logic pushed the demorgan down through the original instead of using parens.

A better solution that both of those would be DRAKE_THROW_UNLESS(isfinite(*lower_limit) || isfinite(*upper_limit));. We are stating the positive condition that must be obtained -- that at least one of the two limits is finite.

---

geometry/meshcat.cc line 773 at r3 (raw file):

        rate_calculator_(params_.realtime_rate_period) {
    DRAKE_THROW_IF(params.port.has_value() && *params.port == 0 &&
                   *params.port >= 1024);

As with my comment on volume_mesh_refiner (see there for deatils), this code no longer aligns with the API documentation:

  @pre We require `port` == 0 || `port` >= 1024.

If the "has_value" tacked on the front of this check is too much pollution, then we should just unwrap to be

if (port.has_value()) {
  DRAKE_THROW_UNLESS(*port == 0 || *port >= 1024);
}