fix: Use the same compiler options for xrpl.libxrpl and all its submodules #5228

ximinez · 2024-12-19T19:27:38Z

High Level Overview of Change

Applies the cmake settings for the xrpl.libxrpl cmake module to the dependency modules introduced by Enforce levelization in libxrpl with CMake #5111.
Changes the Github CI actions to build with the assert and werr options turned on. This will cause CI jobs to fail if a developer introduces a new compiler warning, or causes an assert to fail in release builds.
Includes the OS and compiler version in the linux dependencies jobs in the "check environment" step.

Context of Change

#5111 reorganized the folders in src/libxrpl into dependencies of the xrpl.libxrpl cmake library. Unfortunately, this caused those modules to no longer have the settings that were applied to xrpl.libxrpl.

One of those settings was the -Wno-maybe-uninitialized compiler option, which resulted in new "may be used uninitialized" build warnings. When compiling with derr=TRUE, those warnings became errors, which made the build fail. The first occurence was in the "protocol" module, but there were others.

(Incidentally, this also disabled Antithesis instrumentation (#5042 #5213) in those modules.)

Type of Change

Bug fix (non-breaking change which fixes an issue)

Before / After

Before:

Builds with warnings will pass CI checks.
Build fails when werr cmake option is enabled.

After:

Builds will warnings will fail some or all CI checks.
Build succeeds when werr cmake option is enabled.

Test Plan

This does not directly change any source code or functionality, so as long as the build works, and existing tests pass, all is well.

Suggested commit message

Use the same compiler options for all xrpl / xrpld modules    
* Resolves an issue introduced by #5111, which inadvertently removed the
      -Wno-maybe-uninitialized compiler option from some xrpl.libxrpl modules. This
      resulted in new "may be used uninitialized" build warnings, first
      noticed in the "protocol" module. When compiling with derr=TRUE, those
      warnings became errors, which made the build fail.
* Build CI jobs with "werr" set. Treats warnings as errors so the build will fail, avoiding further problems down the line.
* Also include OS and compiler information in the dependencies environment dump.

* Treats warnings as errors so the build will fail, avoiding further problems down the line. * Also include OS and compiler information in the dependencies environment dump.

* Resolves an issue introduced by XRPLF#5111, which inadvertently removed the -Wno-maybe-uninitialized compiler option from those modules. This resulted in new "may be used uninitialized" build warnings, first noticed in the "protocol" module. When compiling with derr=TRUE, those warnings became errors, which made the build fail.

cmake/RippledCore.cmake

thejohnfreeman · 2024-12-22T01:44:40Z

Compiler options like -Wno-maybe-uninitialized were generally set on INTERFACE library Ripple::opts, which does not have that option set. I think it belongs there.

…bmodules" This reverts commit e103657.

* Resolves an issue introduced by XRPLF#5111, which inadvertently removed the -Wno-maybe-uninitialized compiler option from some modules. This resulted in new "may be used uninitialized" build warnings, first noticed in the "protocol" module. When compiling with derr=TRUE, those warnings became errors, which made the build fail.

codecov · 2025-01-07T01:16:10Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.0%. Comparing base (ff8b9aa) to head (d5bc0dd).
Report is 1 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5228     +/-   ##
=========================================
- Coverage     78.0%   78.0%   -0.0%     
=========================================
  Files          789     789             
  Lines        66953   66955      +2     
  Branches      8107    8110      +3     
=========================================
- Hits         52218   52213      -5     
- Misses       14735   14742      +7

Files with missing lines	Coverage Δ
src/xrpld/app/paths/detail/DirectStep.cpp	`82.5% <100.0%> (ø)`

... and 2 files with indirect coverage changes

* upstream/develop: chore: add macos dependency installation (5233) prefix Uint384 and Uint512 with Hash in server_definitions (5231) refactor: add `rpcName` to `LEDGER_ENTRY` macro (5202)

ximinez · 2025-01-07T23:19:40Z

Note to self: Also need to move the UNITY_BUILD setting to Ripple::opts

Edit Turns out that target properties don't propagate to dependents the way compile definitions and options do. So I added a unity parameter to add_module. I think that doesn't break any expectations.

* Turns out that target properties don't propagate to dependents the way compile definitions and options do.

* upstream/develop: chore: update deprecated Github Actions (5241) XLS-46: DynamicNFT (5048)

cmake/add_module.cmake

This reverts commit 2b12370.

* Removes the need to set the UNITY_BUILD property on each project, except xrpl.libpb, where it needs to be forced OFF.

* upstream/develop: Fix failing assert in `connect` RPC (5235)

Bronek · 2025-01-15T09:04:21Z

With werr enabled and gcc 13, I am getting compilation error:

../src/xrpld/app/paths/detail/DirectStep.cpp: In member function 'uint32_t ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const':
../src/xrpld/app/paths/detail/DirectStep.cpp:339:17: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
  339 |     auto const& field = [this, qDir]() -> SF_UINT32 const& {
      |                 ^~~~~
../src/xrpld/app/paths/detail/DirectStep.cpp:356:6: note: the temporary was destroyed at the end of the full expression '<lambda closure object>ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const::<lambda()>{((const ripple::DirectIPaymentStep*)this), qDir}.ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const::<lambda()>()'
  339 |     auto const& field = [this, qDir]() -> SF_UINT32 const& {
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  340 |         if (qDir == QualityDirection::in)
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This is not new, but given the aim of this PR is to revive the werr option, please consider applying this patch to fix it:

--- a/src/xrpld/app/paths/detail/DirectStep.cpp
+++ b/src/xrpld/app/paths/detail/DirectStep.cpp
@@ -336,22 +336,22 @@ DirectIPaymentStep::quality(ReadView const& sb, QualityDirection qDir) const
     if (!sle)
         return QUALITY_ONE;
 
-    auto const& field = [this, qDir]() -> SF_UINT32 const& {
+    auto const& field = *[this, qDir]() {
         if (qDir == QualityDirection::in)
         {
             // compute dst quality in
             if (this->dst_ < this->src_)
-                return sfLowQualityIn;
+                return &sfLowQualityIn;
             else
-                return sfHighQualityIn;
+                return &sfHighQualityIn;
         }
         else
         {
             // compute src quality out
             if (this->src_ < this->dst_)
-                return sfLowQualityOut;
+                return &sfLowQualityOut;
             else
-                return sfHighQualityOut;
+                return &sfHighQualityOut;
         }
     }();

The identical fix is also in #5224 (thanks to @thejohnfreeman ) but of course that PR will take much longer to finish, review and merge.

Bronek · 2025-01-15T09:26:06Z

With werr option, clang 18 reports a different error, but I suggest fixing it belongs elsewhere (I am working on a better expected polyfill).

In file included from ../src/xrpld/app/ledger/detail/InboundLedger.cpp:20:
In file included from ../src/xrpld/app/ledger/AccountStateSF.h:23:
In file included from ../src/xrpld/app/ledger/AbstractFetchPackContainer.h:24:
In file included from modules/xrpl.libxrpl.basics/xrpl/basics/base_uint.h:28:
In file included from modules/xrpl.libxrpl.basics/xrpl/basics/Expected.h:25:
In file included from /home/LAN/bronek/.conan/data/boost/1.82.0/_/_/package/0a1827e79100ca0d7448c029d363d0a161d9f93e/include/boost/outcome.hpp:32:
In file included from /home/LAN/bronek/.conan/data/boost/1.82.0/_/_/package/0a1827e79100ca0d7448c029d363d0a161d9f93e/include/boost/outcome/iostream_support.hpp:34:
In file included from /home/LAN/bronek/.conan/data/boost/1.82.0/_/_/package/0a1827e79100ca0d7448c029d363d0a161d9f93e/include/boost/outcome/outcome.hpp:34:
In file included from /home/LAN/bronek/.conan/data/boost/1.82.0/_/_/package/0a1827e79100ca0d7448c029d363d0a161d9f93e/include/boost/outcome/result.hpp:34:
In file included from /home/LAN/bronek/.conan/data/boost/1.82.0/_/_/package/0a1827e79100ca0d7448c029d363d0a161d9f93e/include/boost/outcome/boost_result.hpp:36:
In file included from /home/LAN/bronek/.conan/data/boost/1.82.0/_/_/package/0a1827e79100ca0d7448c029d363d0a161d9f93e/include/boost/system/system_error.hpp:9:
In file included from /home/LAN/bronek/.conan/data/boost/1.82.0/_/_/package/0a1827e79100ca0d7448c029d363d0a161d9f93e/include/boost/system/errc.hpp:14:
In file included from /home/LAN/bronek/.conan/data/boost/1.82.0/_/_/package/0a1827e79100ca0d7448c029d363d0a161d9f93e/include/boost/system/detail/error_code.hpp:14:
In file included from /home/LAN/bronek/.conan/data/boost/1.82.0/_/_/package/0a1827e79100ca0d7448c029d363d0a161d9f93e/include/boost/system/detail/error_category.hpp:18:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/functional:64:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algo.h:61:
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_tempbuf.h:263:8: error: 'get_temporary_buffer<std::pair<ripple::SHAMapNodeID, ripple::base_uint<256>>>' is deprecated [-Werror,-Wdeprecated-declarations]
  263 |                 std::get_temporary_buffer<value_type>(_M_original_len));
      |                      ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algo.h:1580:2: note: in instantiation of member function 'std::_Temporary_buffer<__gnu_cxx::__normal_iterator<std::pair<ripple::SHAMapNodeID, ripple::base_uint<256>> *, std::vector<std::pair<ripple::SHAMapNodeID, ripple::base_uint<256>>>>, std::pair<ripple::SHAMapNodeID, ripple::base_uint<256>>>::_Temporary_buffer' requested here
 1580 |         __buf(__first, std::distance(__first, __last));
      |         ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_algo.h:1617:19: note: in instantiation of function template specialization 'std::__stable_partition<__gnu_cxx::__normal_iterator<std::pair<ripple::SHAMapNodeID, ripple::base_uint<256>> *, std::vector<std::pair<ripple::SHAMapNodeID, ripple::base_uint<256>>>>, __gnu_cxx::__ops::_Iter_pred<(lambda at ../src/xrpld/app/ledger/detail/InboundLedger.cpp:778:37)>>' requested here
 1617 |       return std::__stable_partition(__first, __last,
      |                   ^
../src/xrpld/app/ledger/detail/InboundLedger.cpp:777:21: note: in instantiation of function template specialization 'std::stable_partition<__gnu_cxx::__normal_iterator<std::pair<ripple::SHAMapNodeID, ripple::base_uint<256>> *, std::vector<std::pair<ripple::SHAMapNodeID, ripple::base_uint<256>>>>, (lambda at ../src/xrpld/app/ledger/detail/InboundLedger.cpp:778:37)>' requested here
  777 |     auto dup = std::stable_partition(
      |                     ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/stl_tempbuf.h:99:5: note: 'get_temporary_buffer<std::pair<ripple::SHAMapNodeID, ripple::base_uint<256>>>' has been explicitly marked deprecated here
   99 |     _GLIBCXX17_DEPRECATED
      |     ^
/usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/x86_64-linux-gnu/c++/12/bits/c++config.h:119:34: note: expanded from macro '_GLIBCXX17_DEPRECATED'
  119 | # define _GLIBCXX17_DEPRECATED [[__deprecated__]]
      |                                  ^
1 error generated.

ximinez · 2025-01-15T23:06:55Z

This is not new, but given the aim of this PR is to revive the werr option, please consider applying this patch to fix it:

That patch feels really hacky. Then again, any workaround is probably going to feel hacky. I don't have easy access to gcc13 right now, so I'll apply the patch and commit so we can move forward. I'm curious, though, do you get any different results from either:

diff --git a/src/xrpld/app/paths/detail/DirectStep.cpp b/src/xrpld/app/paths/detail/DirectStep.cpp
index 95e64b337b..0577fcccb3 100644
--- a/src/xrpld/app/paths/detail/DirectStep.cpp
+++ b/src/xrpld/app/paths/detail/DirectStep.cpp
@@ -341,17 +341,17 @@ DirectIPaymentStep::quality(ReadView const& sb, QualityDirection qDir) const
         {
             // compute dst quality in
             if (this->dst_ < this->src_)
-                return sfLowQualityIn;
+                return std::ref(sfLowQualityIn);
             else
-                return sfHighQualityIn;
+                return std::ref(sfHighQualityIn);
         }
         else
         {
             // compute src quality out
             if (this->src_ < this->dst_)
-                return sfLowQualityOut;
+                return std::ref(sfLowQualityOut);
             else
-                return sfHighQualityOut;
+                return std::ref(sfHighQualityOut);
         }
     }();

or

diff --git a/src/xrpld/app/paths/detail/DirectStep.cpp b/src/xrpld/app/paths/detail/DirectStep.cpp
index 95e64b337b..b8b94d06ab 100644
--- a/src/xrpld/app/paths/detail/DirectStep.cpp
+++ b/src/xrpld/app/paths/detail/DirectStep.cpp
@@ -336,22 +336,27 @@ DirectIPaymentStep::quality(ReadView const& sb, QualityDirection qDir) const
     if (!sle)
         return QUALITY_ONE;
 
-    auto const& field = [this, qDir]() -> SF_UINT32 const& {
+    auto const& field = [this,
+                         qDir,
+                         lqi = &sfLowQualityIn,
+                         hqi = &sfHighQualityIn,
+                         lqo = &sfLowQualityOut,
+                         hqo = &sfHighQualityOut]() -> SF_UINT32 const& {
         if (qDir == QualityDirection::in)
         {
             // compute dst quality in
             if (this->dst_ < this->src_)
-                return sfLowQualityIn;
+                return lqi;
             else
-                return sfHighQualityIn;
+                return hqi;
         }
         else
         {
             // compute src quality out
             if (this->src_ < this->dst_)
-                return sfLowQualityOut;
+                return lqo;
             else
-                return sfHighQualityOut;
+                return hqo;
         }
     }();

Bronek · 2025-01-16T10:58:06Z

This is not new, but given the aim of this PR is to revive the werr option, please consider applying this patch to fix it:

That patch feels really hacky. Then again, any workaround is probably going to feel hacky. I don't have easy access to gcc13 right now, so I'll apply the patch and commit so we can move forward. I'm curious, though, do you get any different results from either:

diff --git a/src/xrpld/app/paths/detail/DirectStep.cpp b/src/xrpld/app/paths/detail/DirectStep.cpp
index 95e64b337b..0577fcccb3 100644
--- a/src/xrpld/app/paths/detail/DirectStep.cpp
+++ b/src/xrpld/app/paths/detail/DirectStep.cpp
@@ -341,17 +341,17 @@ DirectIPaymentStep::quality(ReadView const& sb, QualityDirection qDir) const
         {
             // compute dst quality in
             if (this->dst_ < this->src_)
-                return sfLowQualityIn;
+                return std::ref(sfLowQualityIn);
             else
-                return sfHighQualityIn;
+                return std::ref(sfHighQualityIn);
         }
         else
         {
             // compute src quality out
             if (this->src_ < this->dst_)
-                return sfLowQualityOut;
+                return std::ref(sfLowQualityOut);
             else
-                return sfHighQualityOut;
+                return std::ref(sfHighQualityOut);
         }
     }();

This fails:

../src/xrpld/app/paths/detail/DirectStep.cpp: In member function 'uint32_t ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const':
../src/xrpld/app/paths/detail/DirectStep.cpp:339:17: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
  339 |     auto const& field = [this, qDir]() -> SF_UINT32 const& {
      |                 ^~~~~
../src/xrpld/app/paths/detail/DirectStep.cpp:356:6: note: the temporary was destroyed at the end of the full expression '<lambda closure object>ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const::<lambda()>{((const ripple::DirectIPaymentStep*)this), qDir}.ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const::<lambda()>()'
  339 |     auto const& field = [this, qDir]() -> SF_UINT32 const& {
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  340 |         if (qDir == QualityDirection::in)
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  341 |         {
      |         ~
  342 |             // compute dst quality in
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~
  343 |             if (this->dst_ < this->src_)
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  344 |                 return std::ref(sfLowQualityIn);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  345 |             else
      |             ~~~~
  346 |                 return std::ref(sfHighQualityIn);
      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  347 |         }
      |         ~

or

diff --git a/src/xrpld/app/paths/detail/DirectStep.cpp b/src/xrpld/app/paths/detail/DirectStep.cpp
index 95e64b337b..b8b94d06ab 100644
--- a/src/xrpld/app/paths/detail/DirectStep.cpp
+++ b/src/xrpld/app/paths/detail/DirectStep.cpp
@@ -336,22 +336,27 @@ DirectIPaymentStep::quality(ReadView const& sb, QualityDirection qDir) const
     if (!sle)
         return QUALITY_ONE;
 
-    auto const& field = [this, qDir]() -> SF_UINT32 const& {
+    auto const& field = [this,
+                         qDir,
+                         lqi = &sfLowQualityIn,
+                         hqi = &sfHighQualityIn,
+                         lqo = &sfLowQualityOut,
+                         hqo = &sfHighQualityOut]() -> SF_UINT32 const& {
         if (qDir == QualityDirection::in)
         {
             // compute dst quality in
             if (this->dst_ < this->src_)
-                return sfLowQualityIn;
+                return lqi;
             else
-                return sfHighQualityIn;
+                return hqi;
         }
         else
         {
             // compute src quality out
             if (this->src_ < this->dst_)
-                return sfLowQualityOut;
+                return lqo;
             else
-                return sfHighQualityOut;
+                return hqo;
         }
     }();

This also fails:

../src/xrpld/app/paths/detail/DirectStep.cpp: In lambda function:
../src/xrpld/app/paths/detail/DirectStep.cpp:349:24: error: invalid initialization of reference of type 'const ripple::SF_UINT32&' {aka 'const ripple::TypedField<ripple::STInteger<unsigned int> >&'} from expression of type 'const ripple::TypedField<ripple::STInteger<unsigned int> >* const'
  349 |                 return lqi;
      |                        ^~~
../src/xrpld/app/paths/detail/DirectStep.cpp:351:24: error: invalid initialization of reference of type 'const ripple::SF_UINT32&' {aka 'const ripple::TypedField<ripple::STInteger<unsigned int> >&'} from expression of type 'const ripple::TypedField<ripple::STInteger<unsigned int> >* const'
  351 |                 return hqi;
      |                        ^~~
../src/xrpld/app/paths/detail/DirectStep.cpp:357:24: error: invalid initialization of reference of type 'const ripple::SF_UINT32&' {aka 'const ripple::TypedField<ripple::STInteger<unsigned int> >&'} from expression of type 'const ripple::TypedField<ripple::STInteger<unsigned int> >* const'
  357 |                 return lqo;
      |                        ^~~
../src/xrpld/app/paths/detail/DirectStep.cpp:359:24: error: invalid initialization of reference of type 'const ripple::SF_UINT32&' {aka 'const ripple::TypedField<ripple::STInteger<unsigned int> >&'} from expression of type 'const ripple::TypedField<ripple::STInteger<unsigned int> >* const'
  359 |                 return hqo;
      |                        ^~~
../src/xrpld/app/paths/detail/DirectStep.cpp: In member function 'uint32_t ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const':
../src/xrpld/app/paths/detail/DirectStep.cpp:339:17: error: possibly dangling reference to a temporary [-Werror=dangling-reference]
  339 |     auto const& field = [this,
      |                 ^~~~~
../src/xrpld/app/paths/detail/DirectStep.cpp:361:6: note: the temporary was destroyed at the end of the full expression '<lambda closure object>ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const::<lambda()>{((const ripple::DirectIPaymentStep*)this), qDir, (& ripple::sfLowQualityIn), (& ripple::sfHighQualityIn), (& ripple::sfLowQualityOut), (& ripple::sfHighQualityOut)}.ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const::<lambda()>()'
  339 |     auto const& field = [this,
      |                         ~~~~~~
  340 |                          qDir,
      |                          ~~~~~
  341 |                          lqi = &sfLowQualityIn,
      |                          ~~~~~~~~~~~~~~~~~~~~~~
  342 |                          hqi = &sfHighQualityIn,
      |                          ~~~~~~~~~~~~~~~~~~~~~~~
  343 |                          lqo = &sfLowQualityOut,
      |                          ~~~~~~~~~~~~~~~~~~~~~~~
  344 |                          hqo = &sfHighQualityOut]() -> SF_UINT32 const& {
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  345 |         if (qDir == QualityDirection::in)
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  346 |         {
      |         ~
  347 |             // compute dst quality in
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~
  348 |             if (this->dst_ < this->src_)
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  349 |                 return lqi;
      |                 ~~~~~~~~~~~
  350 |             else
      |             ~~~~
  351 |                 return hqi;
      |                 ~~~~~~~~~~~
  352 |         }
      |         ~
  353 |         else
      |         ~~~~

However, I think you will like this one better, and it does compile cleanly on gcc 13 (go figure):

diff --git a/src/xrpld/app/paths/detail/DirectStep.cpp b/src/xrpld/app/paths/detail/DirectStep.cpp
index 95e64b337b..ffd500009e 100644
--- a/src/xrpld/app/paths/detail/DirectStep.cpp
+++ b/src/xrpld/app/paths/detail/DirectStep.cpp
@@ -336,7 +336,7 @@ DirectIPaymentStep::quality(ReadView const& sb, QualityDirection qDir) const
     if (!sle)
         return QUALITY_ONE;

-    auto const& field = [this, qDir]() -> SF_UINT32 const& {
+    auto const& field = [&, this]() -> SF_UINT32 const& {
         if (qDir == QualityDirection::in)
         {
             // compute dst quality in

.github/workflows/nix.yml

cmake/RippledInterface.cmake

Bronek

Some minor remarks aside which are for your consideration but aren't blockers, I like this change a lot.

ximinez · 2025-01-16T16:04:59Z

However, I think you will like this one better, and it does compile cleanly on gcc 13 (go figure):

D'oh! What an oversight! I'll change it to [&, this, qDir], since qDir is so small.

This reverts commit 0ec72ec.

Bronek · 2025-01-16T16:17:51Z

However, I think you will like this one better, and it does compile cleanly on gcc 13 (go figure):

D'oh! What an oversight! I'll change it to [&, this, qDir], since qDir is so small.

Annoyingly, this fails with gcc 13, while [&, this] compiles cleanly. Do not ask me why ¯\(ツ)/¯

../src/xrpld/app/paths/detail/DirectStep.cpp: In member function 'uint32_t ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const':
../src/xrpld/app/paths/detail/DirectStep.cpp:339:17: warning: possibly dangling reference to a temporary [-Wdangling-reference]
  339 |     auto const& field = [&, this, qDir]() -> SF_UINT32 const& {
      |                 ^~~~~
../src/xrpld/app/paths/detail/DirectStep.cpp:356:6: note: the temporary was destroyed at the end of the full expression '<lambda closure object>ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const::<lambda()>{((const ripple::DirectIPaymentStep*)this), qDir}.ripple::DirectIPaymentStep::quality(const ripple::ReadView&, ripple::QualityDirection) const::<lambda()>()'
  339 |     auto const& field = [&, this, qDir]() -> SF_UINT32 const& {
      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  340 |         if (qDir == QualityDirection::in)
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  341 |         {
      |         ~
  342 |             // compute dst quality in
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~
  343 |             if (this->dst_ < this->src_)
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  344 |                 return sfLowQualityIn;
      |                 ~~~~~~~~~~~~~~~~~~~~~~
  345 |             else
      |             ~~~~
  346 |                 return sfHighQualityIn;
      |                 ~~~~~~~~~~~~~~~~~~~~~~~
  347 |         }
      |         ~

ximinez · 2025-01-16T20:40:49Z

Commit message:

fix: Use consistent CMake settings for all modules

* Resolves an issue introduced in #5111, which inadvertently removed the
  -Wno-maybe-uninitialized compiler option from some xrpl.libxrpl
  modules. This resulted in new "may be used uninitialized" build
  warnings, first noticed in the "protocol" module. When compiling with
  derr=TRUE, those warnings became errors, which made the build fail.
* Github CI actions will build with the assert and werr options turned
  on. This will cause CI jobs to fail if a developer introduces a new
  compiler warning, or causes an assert to fail in release builds.
* Includes the OS and compiler version in the linux dependencies jobs in
  the "check environment" step.
* Translates the `unity` build option into `CMAKE_UNITY_BUILD` setting.

Issues have been resolved. John is not expected to be available to follow-up.

ximinez added 2 commits December 19, 2024 14:24

Build CI jobs with "werr" set

31ce7c8

* Treats warnings as errors so the build will fail, avoiding further problems down the line. * Also include OS and compiler information in the dependencies environment dump.

ximinez marked this pull request as ready for review December 19, 2024 21:53

ximinez marked this pull request as draft December 19, 2024 21:53

ximinez requested review from Bronek, thejohnfreeman, vlntb and vvysokikh1 and removed request for vlntb December 19, 2024 22:17

ximinez assigned Bronek and vvysokikh1 Dec 19, 2024

ximinez changed the title ~~DRAFT: Build CI jobs with "werr" set~~ Build CI jobs with "werr" set Dec 19, 2024

ximinez marked this pull request as ready for review December 19, 2024 22:23

ximinez changed the title ~~Build CI jobs with "werr" set~~ fix: Use the same compiler options for xrpl.libxrpl and all its submodules Dec 19, 2024

ximinez added CI Continuous Integration Functionality Build System Bug Perf impact not expected Change is not expected to improve nor harm performance. labels Dec 19, 2024

ximinez added this to the 2.4.0 (2025) milestone Dec 19, 2024

godexsoft approved these changes Dec 19, 2024

View reviewed changes

thejohnfreeman requested changes Dec 22, 2024

View reviewed changes

cmake/RippledCore.cmake Outdated Show resolved Hide resolved

ximinez added 2 commits January 6, 2025 18:40

Revert "Use the same compiler options for xrpl.libxrpl and all its su…

f7aaccb

…bmodules" This reverts commit e103657.

Merge remote-tracking branch 'upstream/develop' into pr/werr

a7f4436

* upstream/develop: chore: add macos dependency installation (5233) prefix Uint384 and Uint512 with Hash in server_definitions (5231) refactor: add `rpcName` to `LEDGER_ENTRY` macro (5202)

Add a "unity" param to add_module

2b12370

* Turns out that target properties don't propagate to dependents the way compile definitions and options do.

ximinez requested a review from thejohnfreeman January 9, 2025 03:52

ximinez mentioned this pull request Jan 9, 2025

Proposed 2.4.0-b1 #5223

Closed

1 task

Merge remote-tracking branch 'upstream/develop' into pr/werr

424c1e1

* upstream/develop: chore: update deprecated Github Actions (5241) XLS-46: DynamicNFT (5048)

bthomee requested a review from legleux January 13, 2025 22:48

Merge branch 'develop' into pr/werr

ad25b1c

legleux approved these changes Jan 14, 2025

View reviewed changes

thejohnfreeman previously requested changes Jan 14, 2025

View reviewed changes

cmake/add_module.cmake Outdated Show resolved Hide resolved

ximinez added 3 commits January 14, 2025 20:10

Revert "Add a "unity" param to add_module"

dada970

This reverts commit 2b12370.

Make use of the CMAKE_UNITY_BUILD variable

3b79e1f

* Removes the need to set the UNITY_BUILD property on each project, except xrpl.libpb, where it needs to be forced OFF.

Merge remote-tracking branch 'upstream/develop' into pr/werr

de6f9db

* upstream/develop: Fix failing assert in `connect` RPC (5235)

From @Bronek: fix error in gcc 13

0ec72ec

Bronek reviewed Jan 16, 2025

View reviewed changes

.github/workflows/nix.yml Show resolved Hide resolved

Bronek reviewed Jan 16, 2025

View reviewed changes

cmake/RippledInterface.cmake Outdated Show resolved Hide resolved

Bronek approved these changes Jan 16, 2025

View reviewed changes

ximinez added 3 commits January 16, 2025 11:06

Revert "From @Bronek: fix error in gcc 13"

535efd7

This reverts commit 0ec72ec.

From @Bronek: simpler error fix for gcc 13

699b0ea

Move ENABLE_VOIDSTAR to target_compile_definitions

b2cce3f

fixup! From @Bronek: simpler error fix for gcc 13

d5bc0dd

Merge branch 'develop' into pr/werr

a0067ed

ximinez added the Ready to merge (Author) Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Jan 16, 2025

bthomee approved these changes Jan 16, 2025

View reviewed changes

bthomee merged commit 839d17e into XRPLF:develop Jan 16, 2025
19 checks passed

ximinez mentioned this pull request Jan 16, 2025

Proposed 2.4.0-b2 #5249

Open

5 tasks

ximinez deleted the pr/werr branch January 16, 2025 22:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Use the same compiler options for xrpl.libxrpl and all its submodules #5228

fix: Use the same compiler options for xrpl.libxrpl and all its submodules #5228

ximinez commented Dec 19, 2024 •

edited

Loading

thejohnfreeman commented Dec 22, 2024

codecov bot commented Jan 7, 2025 •

edited

Loading

ximinez commented Jan 7, 2025 •

edited

Loading

Bronek commented Jan 15, 2025

Bronek commented Jan 15, 2025

ximinez commented Jan 15, 2025

Bronek commented Jan 16, 2025

Bronek left a comment

ximinez commented Jan 16, 2025

Bronek commented Jan 16, 2025 •

edited

Loading

ximinez commented Jan 16, 2025

fix: Use the same compiler options for xrpl.libxrpl and all its submodules #5228

fix: Use the same compiler options for xrpl.libxrpl and all its submodules #5228

Conversation

ximinez commented Dec 19, 2024 • edited Loading

High Level Overview of Change

Context of Change

Type of Change

Before / After

Test Plan

Suggested commit message

thejohnfreeman commented Dec 22, 2024

codecov bot commented Jan 7, 2025 • edited Loading

Codecov Report

ximinez commented Jan 7, 2025 • edited Loading

Bronek commented Jan 15, 2025

Bronek commented Jan 15, 2025

ximinez commented Jan 15, 2025

Bronek commented Jan 16, 2025

Bronek left a comment

Choose a reason for hiding this comment

ximinez commented Jan 16, 2025

Bronek commented Jan 16, 2025 • edited Loading

ximinez commented Jan 16, 2025

ximinez commented Dec 19, 2024 •

edited

Loading

codecov bot commented Jan 7, 2025 •

edited

Loading

ximinez commented Jan 7, 2025 •

edited

Loading

Bronek commented Jan 16, 2025 •

edited

Loading