Conversation

liam-x

fix #1710

clang 14 impose a new restriction on __has_declspec_attribute, the argument can not be empty.
llvm/llvm-project#53269

cppresetsdk cpprest_compat.h translate dllimport to empty which cause compilation failure on clang 14.

A workaround by removing define dllimport and add no-unknown-attributes flag to suppress the compilation warning from clang on macOS and Linux.

@liam-xliam-x marked this pull request as ready for review October 14, 2022 04:42
@@ -29,7 +29,9 @@
#else // ^^^ _WIN32 ^^^ // vvv !_WIN32 vvv

#define __declspec(x) __attribute__((x))
#define dllimport
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is a good way to fix this. There are not that many appearances of __declspec in the codebase, and most are already behind visibility macros, the remaining are noinline, noreturn, and novtable. Noreturn can probably be replaced by writing [[noreturn]] directly, noinline should be replaced by something like

#ifdef _MSC_VER
#define CPPREST_NOINLINE __declspec(noinline)
#else
#define CPPREST_NOINLINE __attribute__((noinline))
#endif

and then replacing __declspec(noinline) usages with CPPREST_NOINLINE

the usages with dllimport/dllexport should be replaced by a proper set of visibility macros, cmake can generate them but they are also easy to write, something like:

#ifdef _MSC_VER
    #define CPPREST_API_CLASS
    #if defined(CPPREST_BUILDING) && defined(CPPREST_DLL)
        #define CPPREST_API __declspec(dllexport)
    #elif defined(CPPREST_DLL)
        #define CPPREST_API __declspec(dllimport)
    #else
        #define CPPREST_API
    #endif
#else
    // it might be a good idea to do something special for static libraries here,
    // since this could result in apps exporting symbols to loaded DSOs instead of those
    // DSOs loading cpprestsdk themselves.
    #define CPPREST_API_CLASS __attribute__((visibility("default")))
    #define CPPREST_API __attribute__((visibility("default")))
    #endif
#endif

any classes used as exceptions (or generally if the RTTI information needs exporting) nned to be annotated as CPPREST_API_CLASS, this should be a seperate macro because on windows applying dllexport to a class can export all it's bases, which is extremely annoying

Sign up for free to join this conversation on . Already have an account? Sign in to comment
None yet
None yet

Successfully merging this pull request may close these issues.

cpprest_compat breaks cURL header when building with Clang 14