Fix a case where ripple::Expected returned a json array, not a value

[scottschurr]

High Level Overview of Change

@godexsoft reported a problem with the ripple::Expected implementation and provided a proposed fix. The problem is that there are conditions where Expected invokes the wrong constructor for the expected type. A constructor that takes multiple arguments might be interpreted as an array.

The proposed fix was a minor adjustment to three constructors that replaces the use of curly braces with parens.

A unit test also came along for the ride.

This pull request incorporates the suggested fix and the unit test into the rippled code base.

Context of Change

Clio is using ripple::Expected and had a problem. This change makes Expected useable for Clio.

Type of Change

• Bug fix (non-breaking change which fixes an issue)
• Tests (You added tests for code that already exists, or your new feature included in this PR)

[seelabs]

👍 LGTM

[HowardHinnant]

Subtle!

[HowardHinnant]

I approved too quickly.

Using boost 1.77 I'm getting this link-time error:

[ 16%] Linking CXX executable rippled
Undefined symbols for architecture x86_64:
  "boost::json::value::value(boost::json::value&&)", referenced from:
      ripple::test::Expected_test::run()::'lambda9'()::operator()() const in Expected_test.cpp.o
  "boost::json::value::~value()", referenced from:
      ripple::test::Expected_test::run() in Expected_test.cpp.o
      ripple::test::Expected_test::run()::'lambda9'()::operator()() const in Expected_test.cpp.o
  "boost::json::object::object(boost::json::object&&)", referenced from:
      ripple::test::Expected_test::run()::'lambda9'()::operator()() const in Expected_test.cpp.o
  "boost::json::object::object(std::initializer_list<std::__1::pair<boost::basic_string_view<char, std::__1::char_traits<char> >, boost::json::value_ref> >, unsigned long, boost::json::storage_ptr)", referenced from:
      ripple::test::Expected_test::run()::'lambda9'()::operator()() const in Expected_test.cpp.o
  "boost::json::object::~object()", referenced from:
      ripple::test::Expected_test::run()::'lambda9'()::operator()() const in Expected_test.cpp.o
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [rippled] Error 1
make[1]: *** [CMakeFiles/rippled.dir/all] Error 2
make: *** [all] Error 2

It appears to be a boost error. Investigating...

[scottschurr]

I'm using boost 1_75_0 and it's building for me. I'll be interested to hear what you discover @HowardHinnant.

[HowardHinnant]

I just replicated my link error with boost 1.75. I'm beginning to suspect a compiler or linker error as my machine has been updated to use a beta developer tools (without my consent).

I have another machine I can set up. Moving to that...

[HowardHinnant]

I have a new trusted development machine that correctly compiles, links and unittests this! 🎉 Sorry for the delay.

[scottschurr]

@HowardHinnant, thanks for being thorough. Marked as Passed.

[intelliot]

(not blocking on @godexsoft; just tagging for visibility)

[godexsoft]

LGTM 🚀

[intelliot]

Suggested commit message:

fix(`Expected`): return a value: (#4401)

Fix a case where `ripple::Expected` returned a json array, not a value.

The problem was that `Expected` invoked the wrong constructor for the
expected type, which resulted in a constructor that took multiple
arguments being interpreted as an array.

A proposed fix was provided by @godexsoft, which involved a minor
adjustment to three constructors that replaces the use of curly braces
with parentheses. This makes `Expected` usable for
[Clio](https://github.com/XRPLF/clio).

A unit test is also included to ensure that the issue doesn't occur
again in the future.

[ximinez]

I'm not a fan of this commit message style. I still use https://cbea.ms/git-commit/ as sort of the "git commit message bible".

If the goal is to write shorter subject lines and include the PR number, how about something like:

Fix `Expected` to return a value (#4401)

[scottschurr]

Given the two suggested commit messages, I feel like the one suggested by @ximinez is a bit too terse. I'm fine with the one suggested by @intelliot however. And I appreciate how both of them managed the line lengths 😄

[ximinez]

Given the two suggested commit messages, I feel like the one suggested by @ximinez is a bit too terse. I'm fine with the one suggested by @intelliot however. And I appreciate how both of them managed the line lengths smile

Oh, I should have been clearer - that was just the subject line. The rest of the body was fine.