Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modify variable name to avoid ODR violations with LTO #48

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

parona-source
Copy link
Contributor

@parona-source parona-source commented Apr 28, 2024

  • A different compilation unit hjson_decode also defines hjson::Parser which would conflict with the other definition in hjson_parsenumber. Modifying the variable name in hjson_parsenumber is the path least resistance with only three mentions.
  • As this isn't a part of any public interface this should be only cosmetic for non LTO applications.

You can see the warning with GCC if you enable shared libraries, add warnings for odr and enable lto in flags.

cmake -DCMAKE_CXX_COMPILER=g++ -DCMAKE_CXX_FLAGS="-Wodr -flto" -DBUILD_SHARED_LIBS=ON -B build && cmake --build build

/home/ask/sources/hjson-cpp/src/hjson_decode.cpp:20:7: warning: type ‘struct Parser’ violates the C++ One Definition Rule [-Wodr]
   20 | class Parser {
      |       ^
/home/ask/sources/hjson-cpp/src/hjson_parsenumber.cpp:16:8: note: a different type is defined in another translation unit
   16 | struct Parser {
      |        ^
/home/ask/sources/hjson-cpp/src/hjson_decode.cpp:26:18: note: the first difference of corresponding definitions is field ‘opt’
   26 |   DecoderOptions opt;
      |                  ^
/home/ask/sources/hjson-cpp/src/hjson_parsenumber.cpp:16:8: note: a type with different number of fields is defined in another translation unit
   16 | struct Parser {
      |        ^

* A different compilation unit hjson_decode also defines hjson::Parser
  which would conflict with the other definition in hjson_parsenumber.
  Modifying the variable name in hjson_parsenumber is the path least
  resistance with only three mentions.
* As this isn't a part of any public interface this should be only
  cosmetic for non LTO applications.

Signed-off-by: Alfred Wingate <parona@protonmail.com>
@trobro
Copy link
Member

trobro commented Apr 29, 2024

Thanks for the PR, merging to master.

@trobro trobro merged commit acfab4e into hjson:master Apr 29, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants