[Fix warning] I found some warning when build code fix them？

[MisterRaindrop]

hi @wgtmac
I’ve noticed some compilation warnings. I understand that it’s normal to have warnings during the ongoing development of the code. However, they’re quite annoying to look at. If they haven’t been fixed, I can take care of them right away.
like it:

[  1%] Built target nanoarrow
[  1%] Building CXX object src/iceberg/CMakeFiles/iceberg_shared.dir/catalog/in_memory_catalog.cc.o
[  1%] Building CXX object src/iceberg/CMakeFiles/iceberg_shared.dir/demo.cc.o
[  2%] Building CXX object src/iceberg/CMakeFiles/iceberg_shared.dir/expression/expression.cc.o
[  2%] Building CXX object src/iceberg/CMakeFiles/iceberg_shared.dir/file_reader.cc.o
[  2%] Building CXX object src/iceberg/CMakeFiles/iceberg_shared.dir/json_internal.cc.o
/code/src/iceberg/json_internal.cc: In function 'nlohmann::json_abi_v3_11_3::json iceberg::ToJson(const Type&)':
/code/src/iceberg/json_internal.cc:480:1: warning: control reaches end of non-void function [-Wreturn-type]
  480 | }
      | ^

[MisterRaindrop]

I have see warning like it, It seem code run normally. however, there are compile on warnings. All warnings are of this kind.
Do we need to handle this warning? If we need to handle it, it might be better to throw an exception.
like it:

nlohmann::json ToJson(const Type& type) {
  switch (type.type_id()) {
     case xx:
           xxxx
  }
}

[wgtmac]

Yes, I think we need to fix them. I also noticed that Windows CI has reported similar issues. Do you want to submit a PR?

[MisterRaindrop]

Sure. Let’s discuss how to fix it. I understand that an exception should be thrown because this is not the expected behavior.

[wgtmac]

TBH, I do think this is a bug from the compiler because we have exhausted all cases for the switch statement (please correct me if I was wrong). To fix this, we can simply use Unreachable defined in iceberg/util/unreachable.h in the default branch.

[MisterRaindrop]

It’s possible that the compiler only mechanically checks whether the code conforms to the syntax.

Since normally we can’t reach this point. How can we fix issues like this?

internal::Unreachable();

or

Detailed error information, like

internal::Unreachable("Unknown type id: {}", static_cast<int>(type.type_id()));

Which way do you think is better?

[wgtmac]

I prefer the latter one. Thanks!