2

I implemented a function which generated the first n fibonacci numbers as a coding exercise. To increase the amount of numbers I could generate before overflowing any of the standard integer types I downloaded the BigInt header from github and put it in /usr/include (ubuntu 22.04.4).

No errors are given when everything is kept in a single .cpp file, but I encountered a problem when trying to move all the fibonacci related stuff into its own translation unit. The following code is enough to reproduce the error:

test.cpp:

#include "test_import.h" int main() {}

test_import.h:

#ifndef TEST_IMPORT #define TEST_IMPORT #include <BigInt.hpp> #endif

test_import.cpp:

#include "fibonacci.h"

This generates a multiple definition linker error for every function in <BigInt.hpp> which for example looks like this:

/usr/bin/ld: CMakeFiles/test.dir/test_import.cpp.o: in function bool std::__constant_string_p<char>(char const*)': /usr/include/BigInt.hpp:140: multiple definition of is_valid_number(std::__cxx11::basic_string<char, std::char_traits, std::allocator > const&)'; CMakeFiles/test.dir/test.cpp.o:/usr/include/BigInt.hpp:140: first defined here

But I dont understand why this gives a linker error.

The compiler is: gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0

And here is my CMakeLists.txt if that gives any useful information:

set(PROJECT_NAME chap8) cmake_minimum_required(VERSION 3.22) project(${PROJECT_NAME} VERSION 1.0) set(CMAKE_CXX_STANDARD 17) set(CMAKE_CXX_STANDARD_REQUIRED ON) set(CMAKE_CXX_EXTENSIONS OFF) include_directories("/usr/include/boost_1_85_0") #adds bosst library headers (most does not need to be built) set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin) set(TARGET_NAME test) add_executable(${TARGET_NAME} "${TARGET_NAME}.cpp" "test_import.cpp") target_compile_options(${TARGET_NAME} PRIVATE -std=c++17 -fexceptions -Wall -g -Wmissing-declarations )
Improve this question
8
  • 2
    You have non-inline function definitions in a header file. Functions defined in header files should be inline. Non-inline functions should not go in header files. Commented May 16, 2024 at 20:38
  • The error references line 140 of BigInt.hpp, but no such line exists in the repository you linked. The file has only 107 lines. I found the function in functions/utility.hpp, confirming the above comment. This is a bug in the library. Older compilers may pass this, but it's not valid in modern C++. Consider reporting an issue or fixing it yourself. Note that similar fixes are required in math.hpp and random.hpp Commented May 16, 2024 at 20:50
  • @paddy the mentioned file is available as a release artifact also called bigint.h Commented May 16, 2024 at 20:54
  • @paddy my bad, I should have been more specific with the links. Here is the link specifically to the version (0.5.0) of the header I downloaded: github.com/faheel/BigInt/releases/tag/v0.5.0-dev That header file should include everything. Commented May 16, 2024 at 21:10
  • Yep, found that is mentioned in the README also. This is still a library issue. I'm quite surprised there are no issues or PRs logged for this problem, given the age of this repository and the number of forks and stars on it. It claims support for modern C++ (up to C++17). Commented May 16, 2024 at 21:14

1 Answer 1

Reset to default
1

This is a library issue. Functions defined in a header file should be declared as inline. This stops the linker from trying to resolve multiple definitions across translation units. It would seem that the library has not been designed with multiple-source projects in mind.

To fix this locally, edit the released BigInt.hpp header, and add inline in front of every function definition.

Example:

/* is_valid_number --------------- Checks whether the given string is a valid integer. */ inline bool is_valid_number(const std::string& num) { for (char digit : num) if (digit < '0' or digit > '9') return false; return true; }

You likely need to do that for construtor / member function definitions also. Basically everything.

Example:

/* Default constructor ------------------- */ inline BigInt::BigInt() { value = "0"; sign = '+'; }

So, put on some appropriate music, switch your brain off, copy the text inline onto your clipboard and spend a couple of minutes scrolling and pasting. The problem should go away.

Ideally this should be fixed in the library itself. You can do that yourself by following the guidelines for creating a pull request. If you're not confident to do that, log an issue instead.

The source has been split up into multiple headers in the following locations, which likely all require this fix:

  • include/constructors/*.hpp
  • include/functions/*.hpp
  • include/operators/*.hpp
Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

I am still very new to C++. But I noticed that all the non-inline functions are a part of the global namespace which could lead to some problems. Do you think this is just a poorly written header even though it has a lot of stars on github? (and maybe look into replacements).
I am not confident enough in my current c++ skills to do a pull request. But I will log an issue.
Most functions are fine if they rely on a BigInt parameter. But something like BigInt pow(const long long& base, int exp) is potentially a problem. This seems to be a convenience function that just calls pow(BigInt(base), exp). In my opinion, that function should not be there, or the whole library of utilities should be namespaced. There are a few other similar examples.
The library works in decimal and uses a string representation of numbers. It is basically a toy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.