fix shadowed declarations

[andrei-pavel]

Compiling a third party app which includes headers from an installed libyang-cpp with -Wshadow shows these warnings.

/opt/libyang-cpp/include/libyang-cpp/Value.hpp: In constructor ‘constexpr libyang::Decimal64::Decimal64(int64_t, uint8_t)’:
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:145:70: warning: declaration of ‘digits’ shadows a member of ‘libyang::Decimal64’ [-Wshadow]
  145 |     explicit constexpr Decimal64(const int64_t number, const uint8_t digits)
      |                                                        ~~~~~~~~~~~~~~^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:118:13: note: shadowed declaration is here
  118 |     uint8_t digits;
      |             ^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:145:48: warning: declaration of ‘number’ shadows a member of ‘libyang::Decimal64’ [-Wshadow]
  145 |     explicit constexpr Decimal64(const int64_t number, const uint8_t digits)
      |                                  ~~~~~~~~~~~~~~^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:117:13: note: shadowed declaration is here
  117 |     int64_t number;
      |             ^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp: In constructor ‘constexpr libyang::Decimal64::Decimal64(int64_t, uint8_t)’:
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:145:70: warning: declaration of ‘digits’ shadows a member of ‘libyang::Decimal64’ [-Wshadow]
  145 |     explicit constexpr Decimal64(const int64_t number, const uint8_t digits)
      |                                                        ~~~~~~~~~~~~~~^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:118:13: note: shadowed declaration is here
  118 |     uint8_t digits;
      |             ^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:145:48: warning: declaration of ‘number’ shadows a member of ‘libyang::Decimal64’ [-Wshadow]
  145 |     explicit constexpr Decimal64(const int64_t number, const uint8_t digits)
      |                                  ~~~~~~~~~~~~~~^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:117:13: note: shadowed declaration is here
  117 |     int64_t number;
      |             ^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp: In constructor ‘constexpr libyang::Decimal64::Decimal64(int64_t, uint8_t)’:
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:145:70: warning: declaration of ‘digits’ shadows a member of ‘libyang::Decimal64’ [-Wshadow]
  145 |     explicit constexpr Decimal64(const int64_t number, const uint8_t digits)
      |                                                        ~~~~~~~~~~~~~~^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:118:13: note: shadowed declaration is here
  118 |     uint8_t digits;
      |             ^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:145:48: warning: declaration of ‘number’ shadows a member of ‘libyang::Decimal64’ [-Wshadow]
  145 |     explicit constexpr Decimal64(const int64_t number, const uint8_t digits)
      |                                  ~~~~~~~~~~~~~~^~~~~~
/opt/libyang-cpp/include/libyang-cpp/Value.hpp:117:13: note: shadowed declaration is here
  117 |     int64_t number;
      |             ^~~~~~

While they may be considered harmless, having the warnings show up for each translation unit makes the output verbose and prevents the developer to find shadowed declarations in his own app.

There are no other warnings shown in the other repos libyang, sysrepo, or sysrepo-cpp. These are the only ones that I get.

Was wondering whether you're willing to accept this change for a better compiling experience. Tried to respect the syntax from other classes of having m_ prepended to members.

[jktjkt]

These members are public (and are meant to be directly accessible to users), so naming them m_foo wouldn't be the right thing. Personally, I'm on the "this is a bad warning" side, but considering that the GCC bug about this one has been open for six years, I think it's reasonable to have our headers -Wshadow-clean today :). Can you please submit a patch (preferably via Gerrit, but an updated PR is OK as well) which renames the constructor parameters to foo_?

[syyyr]

Looks good to me.

[andrei-pavel]

Updated this MR and tried to attach a patch file, but GitHub tells me it's an unsupported file type. Here it is copy-pasted:

diff --git a/include/libyang-cpp/Value.hpp b/include/libyang-cpp/Value.hpp
index ce95730..d999cb3 100644
--- a/include/libyang-cpp/Value.hpp
+++ b/include/libyang-cpp/Value.hpp
@@ -142,9 +142,9 @@ struct LIBYANG_CPP_EXPORT Decimal64 {
         static_assert(digits <= 18);
         return Decimal64{impl::llround(value * impl::pow10int(digits)), digits};
     }
-    explicit constexpr Decimal64(const int64_t number, const uint8_t digits)
-        : number(number)
-        , digits(digits)
+    explicit constexpr Decimal64(const int64_t number_, const uint8_t digits_)
+        : number(number_)
+        , digits(digits_)
     {
     }

Also tried to push to that instance of Gerrit, but I can't get past authentication, neither from CLI, nor from web UI.

I'd appreciate it if you can take it from here,

Thank you!

[jktjkt]

Thanks for the updated PR (no need to attach patches or anything like that). Sorry for the authentication issue -- I'll investigate later.

In the meanwhile, I've uploaded your patch on your behalf (https://gerrit.cesnet.cz/c/CzechLight/libyang-cpp/+/5911). Once it passes CI, it will land. Thanks!