Fix module-import-in-extern-c compiler error

[zacharykeyessonos]

Description

This fixes Mbed-TLS/mbedtls#10490.

This fixes a compiler error when building C++ projects that use the 'modules' C++ language feature consume Mbed TLS. The specific error is module-import-in-extern-c, which occurs when #includes occur within extern C blocks.

Example:

$ clang-17 -std=c++20 -fmodules -Itf-psa-crypto/include -Itf-psa-crypto/drivers/builtin/include -o sample sample.cpp
In file included from sample.cpp:8:
In file included from tf-psa-crypto/drivers/builtin/include/mbedtls/private/chachapoly.h:44:
tf-psa-crypto/drivers/builtin/include/mbedtls/private/chacha20.h:26:1: error: import of C++ module '_Builtin_stdint' appears within extern "C" language linkage specification [-Wmodule-import-in-extern-c]
   26 | #include <stdint.h>
      | ^
tf-psa-crypto/drivers/builtin/include/mbedtls/private/chachapoly.h:35:1: note: extern "C" language linkage specification begins here
   35 | extern "C" {
      | ^
1 error generated.

This seems to primarily be an issue with clang. See https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-fmodules

PR checklist

• changelog entry added
• framework PR not required
• mbedtls development PR provided Fix module-import-in-extern-c compiler error mbedtls#10488
• mbedtls 3.6 PR provided Fix module-import-in-extern-c compiler error (3.6) mbedtls#10489
• tests Sample C++ file that includes all affected headers compiles

[gilles-peskine-arm]

Thank you very much for raising and fixing this!

Would you mind filing an issue with a description of the problem? That lets people know that it's a known problem and whether it's been fixed already.

changelog not required because: ChangeLog.d/00README.md says fixes for compiler warnings are not generally required.

We often don't add changelog entries about compiler warnings because compilers are sometimes weirdly picky, and we don't want to create the expectation that we'll treat every warning from every version of every compiler as a bug. But this is a different, as far as I understand: our header files are not compatible with modern C++, it's not just something you can silence with -Wno-error=..., right? So I think fixing this incompatibility is a bug fix or a new feature (depending on how much one can expect that a C library works out of the box with C++-with-modules).

Many of the maintainers of Mbed TLS and TF-PSA-Crypto, me included, are not very familiar with C++. For example, I know you need to put extern "C" around declarations of functions, but I don't know the precise rules about what else must, can or cannot be wrapped with extern "C". So it would be good to have a test for that. We have a test that tries to build a trivial C++ programs that includes our headers: component_test_full_clang, and the program is cpp_dummy_build.cpp. Can we adapt this test to catch this problem? I guess by changing the build flags in CMakeLists.txt? By default we use an older compiler, but I guess we would need a more recent compiler there?

[zacharykeyessonos]

Would you mind filing an issue with a description of the problem? That lets people know that it's a known problem and whether it's been fixed already.

Done! Mbed-TLS/mbedtls#10490

it's not just something you can silence with -Wno-error=..., right? So I think fixing this incompatibility is a bug fix or a new feature (depending on how much one can expect that a C library works out of the box with C++-with-modules).

I think this actually can be ignored by adding -Wno-module-import-in-extern-c. I'm happy to add a changelog entry for this though.

Can we adapt this test to catch this problem? I guess by changing the build flags in CMakeLists.txt? By default we use an older compiler, but I guess we would need a more recent compiler there?

It looks like simply adding target_compile_options(cpp_dummy_build PRIVATE -fmodules) to the cpp_dummy_build_cpp build exposes the bug. I hit this bug with clang-13, so I think older versions of clang will hit this.

[zacharykeyessonos]

Hi @gilles-peskine-arm — Let me know if there are other changes you thing I should make. Thanks!