Jens Geyer created THRIFT-6235:
----------------------------------

             Summary: Compiler unit tests fail to link when the Go generator is 
disabled
                 Key: THRIFT-6235
                 URL: https://issues.apache.org/jira/browse/THRIFT-6235
             Project: Thrift
          Issue Type: Bug
          Components: Compiler (General), Build Process
    Affects Versions: 0.24.0, 0.23.0
            Reporter: Jens Geyer


h2. Symptom

The compiler unit test binary {{thrift_compiler_tests}} fails to link whenever 
the Go generator is disabled:

{noformat}
undefined reference to 
`t_go_generator::get_publicized_name_and_def_value(t_field*, 
std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> 
>*, t_const_value**) const'
undefined reference to `t_go_generator::is_pointer_field(t_field*, bool)'
undefined reference to `t_go_generator::is_container_keyed_map(t_type*)'
{noformat}

Two configurations disable it:

* configuring {{compiler/cpp/tests}} as a project of its own, with its default 
options;
* a top-level CMake build with {{-DTHRIFT_COMPILER_GO=OFF}}. {{BUILD_TESTING}} 
is on by default, so the {{thrift}} executable still links, but 
{{thrift_compiler_tests}} does not.

Reproduced with GCC on Ubuntu 22.04 (the {{build/docker/ubuntu-jammy}} image) 
on master @ {{b0b31cfa0}}, and on the 0.24.0 and 0.23.0 tags. The tags report 
the first two symbols; master adds the third.

CI does not catch this. The _Build with CMake_ workflow and the MSVC build both 
configure from the top level and leave Go enabled.

h2. Cause

{{compiler/cpp/CMakeLists.txt}} and {{compiler/cpp/tests/CMakeLists.txt}} each 
define a {{THRIFT_ADD_VALIDATOR_COMPILER}} macro, but the two macros key on 
different options:

||File||Option that adds {{go_validator_generator.cc}}||
|{{compiler/cpp/CMakeLists.txt}}|{{THRIFT_COMPILER_GO}}, which also adds 
{{t_go_generator.cc}}|
|{{compiler/cpp/tests/CMakeLists.txt}}|{{THRIFT_VALIDATOR_COMPILER_GO}}, a 
separate option that defaults to {{ON}}|

{{go_validator_generator.cc}} calls members of {{t_go_generator}} that are 
defined in {{t_go_generator.cc}}. In the tests project, the validator can 
therefore be built without the generator it calls into. With that project's 
defaults this always happens, because Go defaults to {{OFF}} there.

The test binary links its library as a whole archive, so that the 
self-registering generators are kept. That pulls in the validator's object 
file, and with it the unresolved references.

The first two calls came with THRIFT-5423. THRIFT-2063 added the call to 
{{is_container_keyed_map()}}.

h2. Proposed fix

Key the macro in {{compiler/cpp/tests/CMakeLists.txt}} on the same option as 
the one in {{compiler/cpp/CMakeLists.txt}}:

{code}
 macro(THRIFT_ADD_VALIDATOR_COMPILER name description initial)
-    string(TOUPPER "THRIFT_VALIDATOR_COMPILER_${name}" enabler)
+    string(TOUPPER "THRIFT_COMPILER_${name}" enabler)
{code}

With that line changed on master, all three configurations link and pass (326 
assertions in 22 test cases):

* the standalone project with its defaults;
* the top-level build with {{-DTHRIFT_COMPILER_GO=OFF}};
* the top-level build with its defaults, which is what CI uses.

_Drafted with AI assistance (Claude Opus 5); every claim above was verified by 
running the builds described._




--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to