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

Mark namespace scope constexpr variable 'buffer_size' inline. #4084

Merged
merged 2 commits into from
Jul 24, 2024

Conversation

kamrann
Copy link
Contributor

@kamrann kamrann commented Jul 23, 2024

This tiny change works around possible clang bug which currently prevents compilation on clang with modules and FMT_ATTACH_TO_GLOBAL_MODULE. From my understanding, std library implementations changed such constexpr namespace scope variables to be inline after C++17, so I think this is perhaps a good change regardless of whether clang is wrong here or not (from my limited understanding, I think it protects against possible ODR violations and has no downside).

With both this and #4083, I can successfully build fmt with modules + FMT_ATTACH_TO_GLOBAL_MODULE on Windows+MSVC, Windows+Clang and Linux+Clang.

@vitaut
Copy link
Contributor

vitaut commented Jul 23, 2024

Similarly to the other case, could you post the full error message?

@kamrann
Copy link
Contributor Author

kamrann commented Jul 24, 2024

For reference.

Command line: C:/LLVM/build-18x/Release/bin/clang++ -DFMT_ATTACH_TO_GLOBAL_MODULE -DFMT_MODULE -IC:\Coding\Build2\third-party-packaging\build2-packaging\fmt\fmt\fmt/include -IC:\Coding\Build2\third-party-packaging\build2-packaging\build\clang-release\fmt -IC:\Coding\Build2\third-party-packaging\build2-packaging\fmt\fmt -std=c++2b -D__cpp_modules=201907L -Wno-unqualified-std-cast-call -fdiagnostics-color -fansi-escape-codes -finput-charset=UTF-8 -Xclang -fexternc-nounwind -D_MT -D_DLL -Xclang -fmodules-embed-all-files -fmodule-output=..\build\clang-release\fmt\fmt\src\fmt.lib.pcm -o ..\build\clang-release\fmt\fmt\src\fmt.lib.pcm.obj -c -x c++-module C:\Coding\Build2\third-party-packaging\build2-packaging\fmt\fmt\fmt\src\fmt.cc

Error output:

In file included from C:\Coding\Build2\third-party-packaging\build2-packaging\fmt\fmt\fmt\src\fmt.cc:113:
C:\Coding\Build2\third-party-packaging\build2-packaging\fmt\fmt\fmt/include\fmt/os.h:386:16: error: declaration of 'buffer_size' with internal linkage cannot be exported
  386 | constexpr auto buffer_size = detail::buffer_size();
      |                ^
C:\Coding\Build2\third-party-packaging\build2-packaging\fmt\fmt\fmt/include\fmt/os.h:77:1: note: export block begins here
   77 | FMT_BEGIN_EXPORT
      | ^
C:\Coding\Build2\third-party-packaging\build2-packaging\fmt\fmt\fmt\src\fmt.cc:89:26: note: expanded from macro 'FMT_BEGIN_EXPORT'
   89 | #define FMT_BEGIN_EXPORT export {
      |                          ^

@vitaut vitaut merged commit 31b3c32 into fmtlib:master Jul 24, 2024
43 checks passed
@vitaut
Copy link
Contributor

vitaut commented Jul 24, 2024

Thanks for the fix!

@kamrann kamrann deleted the pr/inline-constexpr-var branch September 20, 2024 03:33
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