proxy-types: fix clang-tidy EnumCastOutOfRange error

Reported by fanquake <[email protected]>
bitcoin/bitcoin#31802 (comment)
https://cirrus-ci.com/task/6552721135763456
https://api.cirrus-ci.com/v1/task/6552721135763456/logs/ci.log

include/mp/type-number.h:55:32: error: The value '0' provided to the cast expression is not in the valid range of values for 'FooEnum' [clang-analyzer-optin.core.EnumCastOutOfRange,-warnings-as-errors]
   55 |     return read_dest.construct(static_cast<LocalType>(input.get()));
      |                                ^
test/mp/test/foo.h:26:12: note: enum declared here
   26 | enum class FooEnum : uint8_t { ONE = 1, TWO = 2, };
      | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/test/mp/test/foo.capnp.proxy-server.c++:63:12: note: Calling 'serverInvoke<mp::ProxyServer<mp::test::messages::FooInterface>, capnp::CallContext<mp::test::messages::FooInterface::PassEnumParams, mp::test::messages::FooInterface::PassEnumResults>, mp::ServerField<1, mp::Accessor<mp::foo_fields::Arg, 1>, mp::ServerRet<mp::Accessor<mp::foo_fields::Result, 2>, mp::ServerCall>>>'
   63 |     return serverInvoke(*this, call_context, MakeServerField<1, Accessor<foo_fields::Arg, FIELD_IN>>(Make<ServerRet, Accessor<foo_fields::Result, FIELD_OUT>>(ServerCall())));
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:739:16: note: Calling 'ReplaceVoid<(lambda at include/mp/proxy-types.h:739:28), (lambda at include/mp/proxy-types.h:740:13)>'
  739 |         return ReplaceVoid([&]() { return fn.invoke(server_context, ArgList()); },
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  740 |             [&]() { return kj::Promise<CallContext>(kj::mv(call_context)); })
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:700:19: note: 'is_same_v' is true
  700 |     if constexpr (std::is_same_v<decltype(fn()), void>) {
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:700:5: note: Taking true branch
  700 |     if constexpr (std::is_same_v<decltype(fn()), void>) {
      |     ^
include/mp/proxy-types.h:701:9: note: Calling 'operator()'
  701 |         fn();
      |         ^~~~
include/mp/proxy-types.h:739:43: note: Calling 'ServerField::invoke'
  739 |         return ReplaceVoid([&]() { return fn.invoke(server_context, ArgList()); },
      |                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:563:16: note: Calling 'PassField<mp::Accessor<mp::foo_fields::Arg, 1>, mp::test::FooEnum, mp::ServerInvokeContext<mp::ProxyServer<mp::test::messages::FooInterface>, capnp::CallContext<mp::test::messages::FooInterface::PassEnumParams, mp::test::messages::FooInterface::PassEnumResults>>, const mp::ServerRet<mp::Accessor<mp::foo_fields::Result, 2>, mp::ServerCall> &, mp::TypeList<>>'
  563 |         return PassField<Accessor>(Priority<2>(),
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  564 |             typename Split<argc, ArgTypes>::First(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  565 |             server_context,
      |             ~~~~~~~~~~~~~~~
  566 |             this->parent(),
      |             ~~~~~~~~~~~~~~~
  567 |             typename Split<argc, ArgTypes>::Second(),
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  568 |             std::forward<Args>(args)...);
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:304:5: note: Calling 'MaybeReadField<mp::TypeList<mp::test::FooEnum>, mp::InvokeContext &, mp::StructField<mp::Accessor<mp::foo_fields::Arg, 1>, const mp::test::messages::FooInterface::PassEnumParams::Reader>, mp::ReadDestEmplace<mp::test::FooEnum, (lambda at include/mp/proxy-types.h:305:83)>>'
  304 |     MaybeReadField(std::integral_constant<bool, Accessor::in>(), TypeList<ArgType>(), invoke_context,
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  305 |         Make<StructField, Accessor>(params), ReadDestEmplace(TypeList<ArgType>(), [&](auto&&... args) -> auto& {
      |         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  306 |             param.emplace(std::forward<decltype(args)>(args)...);
      |             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  307 |             return *param;
      |             ~~~~~~~~~~~~~~
  308 |         }));
      |         ~~~
include/mp/proxy-types.h:276:5: note: Calling 'ReadField<mp::test::FooEnum, mp::InvokeContext &, mp::StructField<mp::Accessor<mp::foo_fields::Arg, 1>, const mp::test::messages::FooInterface::PassEnumParams::Reader>, mp::ReadDestEmplace<mp::test::FooEnum, (lambda at include/mp/proxy-types.h:305:83)>>'
  276 |     ReadField(std::forward<Args>(args)...);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:175:12: note: Calling 'CustomReadField<mp::test::FooEnum, mp::StructField<mp::Accessor<mp::foo_fields::Arg, 1>, const mp::test::messages::FooInterface::PassEnumParams::Reader>, mp::ReadDestEmplace<mp::test::FooEnum, (lambda at include/mp/proxy-types.h:305:83)>>'
  175 |     return CustomReadField(TypeList<RemoveCvRef<LocalTypes>...>(), Priority<2>(), std::forward<Args>(args)...);
      |            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/type-number.h:55:55: note: Calling 'StructField::get'
   55 |     return read_dest.construct(static_cast<LocalType>(input.get()));
      |                                                       ^~~~~~~~~~~
include/mp/proxy-types.h:40:41: note: Calling 'Arg::get'
   40 |     decltype(auto) get() const { return Accessor::get(this->m_struct); }
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/test/mp/test/foo.capnp.proxy.h:198:82: note: Calling 'Reader::getArg'
  198 |     template<typename S> static auto get(S&& s) -> decltype(s.getArg()) { return s.getArg(); }
      |                                                                                  ^~~~~~~~~~
build/test/mp/test/foo.capnp.h:7042:10: note: Calling 'StructReader::getDataField'
 7042 |   return _reader.getDataField< ::int32_t>(
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 7043 |       ::capnp::bounded<0>() * ::capnp::ELEMENTS);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/capnp/layout.h:1099:7: note: Assuming the condition is false
 1099 |   if ((offset + ONE * ELEMENTS) * capnp::bitsPerElement<T>() <= dataSize) {
      |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/capnp/layout.h:1099:3: note: Taking false branch
 1099 |   if ((offset + ONE * ELEMENTS) * capnp::bitsPerElement<T>() <= dataSize) {
      |   ^
/nix/store/46kiq9naswgbqfc14kc9nxcbgd0rv0m2-capnproto-1.1.0/include/capnp/layout.h:1102:5: note: Returning zero
 1102 |     return static_cast<T>(0);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~
build/test/mp/test/foo.capnp.h:7042:10: note: Returning from 'StructReader::getDataField'
 7042 |   return _reader.getDataField< ::int32_t>(
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 7043 |       ::capnp::bounded<0>() * ::capnp::ELEMENTS);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/test/mp/test/foo.capnp.h:7042:3: note: Returning zero
 7042 |   return _reader.getDataField< ::int32_t>(
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 7043 |       ::capnp::bounded<0>() * ::capnp::ELEMENTS);
      |       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
build/test/mp/test/foo.capnp.proxy.h:198:82: note: Returning from 'Reader::getArg'
  198 |     template<typename S> static auto get(S&& s) -> decltype(s.getArg()) { return s.getArg(); }
      |                                                                                  ^~~~~~~~~~
build/test/mp/test/foo.capnp.proxy.h:198:75: note: Returning zero
  198 |     template<typename S> static auto get(S&& s) -> decltype(s.getArg()) { return s.getArg(); }
      |                                                                           ^~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:40:41: note: Returning from 'Arg::get'
   40 |     decltype(auto) get() const { return Accessor::get(this->m_struct); }
      |                                         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/proxy-types.h:40:34: note: Returning zero
   40 |     decltype(auto) get() const { return Accessor::get(this->m_struct); }
      |                                  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
include/mp/type-number.h:55:55: note: Returning from 'StructField::get'
   55 |     return read_dest.construct(static_cast<LocalType>(input.get()));
      |                                                       ^~~~~~~~~~~
include/mp/type-number.h:55:32: note: The value '0' provided to the cast expression is not in the valid range of values for 'FooEnum'
   55 |     return read_dest.construct(static_cast<LocalType>(input.get()));
      |                                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Author: ryanofsky

.clang-tidy

@@ -27,6 +27,7 @@ readability-const-return-type,
 readability-redundant-declaration,
 readability-redundant-string-init,
 clang-analyzer-core.StackAddressEscape,
+clang-analyzer-optin.core.EnumCastOutOfRange,
 '
 HeaderFilterRegex: '.'
 WarningsAsErrors: '*'

include/mp/type-number.h

@@ -52,6 +52,12 @@ decltype(auto) CustomReadField(TypeList<LocalType>,
     ReadDest&& read_dest,
     typename std::enable_if<std::is_enum<LocalType>::value>::type* enable = nullptr)
 {
+    // Disable clang-tidy out-of-range enum value check which triggers when
+    // using an enum type that does not have a 0 value. The check correctly
+    // triggers when it detects that Cap'n Proto returns 0 when reading an
+    // integer field that is unset. But the warning is spurious because the
+    // corresponding BuildField call should never leave the field unset.
+    // NOLINTNEXTLINE(clang-analyzer-optin.core.EnumCastOutOfRange)
     return read_dest.construct(static_cast<LocalType>(input.get()));
 }