[cleanup] Suppress MSVC Warning in Generated C++ Files

[tusharb86]

This change adds a static_cast to parse_function_generator.cc so that generated C++ files do not emit a warning (C4244 – 'conversion' conversion from 'type1' to 'type2', possible loss of data) when built with MSVC.

This is related to the closed PR #10042.

[tusharb86]

It looks like this is failing because descriptor.pb.cc needs to be regenerated due to the change. (I couldn't find this from any of the logs of the builds - maybe I'm missing something?) I was able to hit this locally after running the full test suite on Linux. Here's the error message from the BootstrapTest.GeneratedFilesMatch test:

google/protobuf/descriptor.pb.cc needs to be regenerated.  Please run generate_descriptor_proto.sh. Then add this file to your CL.

I have tried running generate_descriptor_proto.sh, but I get the following error:

./generate_descriptor_proto.sh
Updating descriptor protos...
Round 1
make: *** No rule to make target 'protoc'.  Stop.
Failed to build protoc.

I'm running VSCode with the CMake extension, using the GCC 9.3.0 toolkit on Ubuntu 20.04. I've configured all projects and built all projects - I see a protoc executable under the build directory. Is there something I'm missing to run this? Any tips @acozzette, @fowles? Thanks!

[acozzette]

@tusharb86 That script seems to assume that you have already run ./autogen.sh; ./configure, so if you do that then it should work. Let me send out a fix for this though, because we are about to delete our autotools build and this will soon stop working.

[acozzette]

Actually I just noticed that there's a fix here that will be submitted soon as part of the autotools removal.

[tusharb86]

Got it - thanks for the info @acozzette! I tried running ./autogen.sh; ./configure, but get the following errors:

./autogen.sh; ./configure
+ test -d third_party/googletest
+ mkdir -p third_party/googletest/m4
+ autoreconf -f -i -Wall,no-obsolete
./autogen.sh: 41: autoreconf: not found
bash: ./configure: No such file or directory

Should I just wait for the other PR to go through before I can merge this one? Is there another way to work around this, so I can wrap up this PR?

[acozzette]

@tusharb86 Installing autoconf should fix that error. You could wait for the other PR to be submitted, but that will require Bazel to be installed. We may want to eventually make the script work with just CMake, but I would guess that installing autoconf will be probably be the fastest way to unblock this.

[tusharb86]

@tusharb86 Installing autoconf should fix that error. You could wait for the other PR to be submitted, but that will require Bazel to be installed. We may want to eventually make the script work with just CMake, but I would guess that installing autoconf will be probably be the fastest way to unblock this.

I probably should have read the error - sorry! That got me a bit further, but now I'm hitting this:

configure.ac:30: error: possibly undefined macro: AC_PROG_LIBTOOL
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
autoreconf: /usr/bin/autoconf failed with exit status: 1

[tusharb86]

@tusharb86 Installing autoconf should fix that error. You could wait for the other PR to be submitted, but that will require Bazel to be installed. We may want to eventually make the script work with just CMake, but I would guess that installing autoconf will be probably be the fastest way to unblock this.

I probably should have read the error - sorry! That got me a bit further, but now I'm hitting this:

configure.ac:30: error: possibly undefined macro: AC_PROG_LIBTOOL
      If this token and others are legitimate, please use m4_pattern_allow.
      See the Autoconf documentation.
autoreconf: /usr/bin/autoconf failed with exit status: 1

And this one requires installing libtool :) Looks like that worked. I'll hopefully have an update to the PR shortly.

[tusharb86]

@acozzette - I checked that the tests now succeed. However, it looks like there are still Kokoro build failures. Clicking on "Details", I'm not seeing anything in the logs to indicate what is wrong. I'm just seeing that the "presubmit" failed. Am I missing something? How do I investigate further?

[acozzette]

@tusharb86 The test failures are still complaining about descriptor.pb.cc not being up to date. Is it possible that there are additional changes that need to be reflected in descriptor.pb.cc?

[tusharb86]

@acozzette I'm not seeing any other changes to descriptor.pb.cc. I tried re-running ./autogen.sh; ./configure and ./generate_descriptor_proto.sh, but there were no other changes made. I've also explicitly run the previously failing test and it now succeeds:

./tests --gtest_filter=*BootstrapTest.GeneratedFilesMatch*
Running main() from gmock_main.cc
Note: Google Test filter = *BootstrapTest.GeneratedFilesMatch*
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from BootstrapTest
[ RUN      ] BootstrapTest.GeneratedFilesMatch
[       OK ] BootstrapTest.GeneratedFilesMatch (126 ms)
[----------] 1 test from BootstrapTest (126 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (126 ms total)
[  PASSED  ] 1 test.

Is there another test that is failing? Also, is there any way for me to look at the logs of the failures or to reproduce the issue locally? Would you be able to help with getting this over the finish line? Thanks!

[acozzette]

The bootstrap test seems to be the only one that's failing. You should be able to see the error logs from the CI runs unless this functionality somehow stopped working. Here is the relevant output:

With diff:
@@ -11061,7 +11061,7 @@
       case 5:
         if (PROTOBUF_PREDICT_TRUE(static_cast<uint8_t>(tag) == 40)) {
-          uint32_t val = ::PROTOBUF_NAMESPACE_ID::internal::ReadVarint32(&ptr);
+          uint64_t val = ::PROTOBUF_NAMESPACE_ID::internal::ReadVarint64(&ptr);
           CHK_(ptr);
-          if (PROTOBUF_PREDICT_TRUE(::PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo_Annotation_Semantic_IsValid(static_cast<int>(val)))) {
+          if (PROTOBUF_PREDICT_TRUE(::PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo_Annotation_Semantic_IsValid(val))) {
             _internal_set_semantic(static_cast<::PROTOBUF_NAMESPACE_ID::GeneratedCodeInfo_Annotation_Semantic>(val));
           } else {

google/protobuf/[descriptor.pb.cc](https://www.google.com/url?q=http://descriptor.pb.cc&sa=D) needs to be regenerated.  Please run generate_descriptor_proto.sh. Then add this file to your CL.
[  FAILED  ] BootstrapTest.GeneratedFilesMatch (5011 ms)

This looks to me like a real error since I would expect that we do need an update around line 11063 in descriptor.pb.cc. Maybe you have another local change that needs to be pushed to GitHub to fix the test?

[tusharb86]

@acozzette I now see what the issue is. My branch is based off of v3.21.3. main has some updates to descriptor.pb.cc which I do not have locally, so I'll need to rebase my branch onto the tip of main.

[tusharb86]

@acozzette Sorry about that! I've regenerated the files on the tip of main and rebuilt/retested successfully. Hopefully, that's it! 🤞

[tusharb86]

@acozzette - Now that all checks have passed, is there anything left from my end to do for the PR?

[acozzette]

I had to fix a merge conflict and re-run the tests but I will merge this assuming the tests pass.

[acozzette]

@tusharb86 Thanks for the fix!

[ssainz]

Hi @acozzette , do you know if this change will make it to the 21.x branch or is it for 22.x branch exclusively?

[acozzette]

Since it just fixes a warning, I think we will probably not release it on 21.x and so it will likely be released first with 22.0.