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.

Sign up for GitHub

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

workaround clang 14 __has_declspec_attribute restriction #1723

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

workaround clang 14 __has_declspec_attribute restriction #1723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a77ac76
workaround clang 14 __has_declspec_attribute restriction
liam-x Oct 7, 2022
ccf78f2
remove define dllimport
liam-x Oct 8, 2022
d497a42
suppress no-unknown-attributes on macOS
liam-x Oct 8, 2022
dc51bc7
add few suppress flags on Linux side
liam-x Oct 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Release/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,12 +147,12 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR IOS)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-pedantic -Wno-attributes -Wno-pointer-arith")
elseif(CMAKE_SYSTEM_NAME MATCHES "Linux")
set(WARNINGS -Wall -Wextra -Wcast-qual -Wconversion -Wformat=2 -Winit-self -Winvalid-pch -Wmissing-format-attribute -Wmissing-include-dirs -Wpacked -Wredundant-decls)
set(LINUX_SUPPRESSIONS -Wno-overloaded-virtual -Wno-sign-conversion -Wno-deprecated -Wno-unknown-pragmas -Wno-reorder -Wno-char-subscripts -Wno-switch -Wno-unused-parameter -Wno-unused-variable -Wno-deprecated -Wno-unused-value -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-unused-function -Wno-sign-compare -Wno-shorten-64-to-32 -Wno-unused-local-typedefs)
set(LINUX_SUPPRESSIONS -Wno-overloaded-virtual -Wno-sign-conversion -Wno-deprecated -Wno-unknown-pragmas -Wno-reorder -Wno-char-subscripts -Wno-switch -Wno-unused-parameter -Wno-unused-variable -Wno-deprecated -Wno-unused-value -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-unused-function -Wno-sign-compare -Wno-shorten-64-to-32 -Wno-unused-local-typedefs -Wno-unused-but-set-parameter -Wno-unknown-attributes -Wno-null-pointer-subtraction)
set(WARNINGS ${WARNINGS} ${LINUX_SUPPRESSIONS})
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-return-type-c-linkage -Wno-unneeded-internal-declaration")
else()
set(WARNINGS -Wall -Wextra -Wcast-qual -Wconversion -Wformat=2 -Winit-self -Winvalid-pch -Wmissing-format-attribute -Wmissing-include-dirs -Wpacked -Wredundant-decls)
set(OSX_SUPPRESSIONS -Wno-overloaded-virtual -Wno-sign-conversion -Wno-deprecated -Wno-unknown-pragmas -Wno-reorder -Wno-char-subscripts -Wno-switch -Wno-unused-parameter -Wno-unused-variable -Wno-deprecated -Wno-unused-value -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-unused-function -Wno-sign-compare -Wno-shorten-64-to-32 -Wno-unused-local-typedefs)
set(OSX_SUPPRESSIONS -Wno-overloaded-virtual -Wno-sign-conversion -Wno-deprecated -Wno-unknown-pragmas -Wno-reorder -Wno-char-subscripts -Wno-switch -Wno-unused-parameter -Wno-unused-variable -Wno-deprecated -Wno-unused-value -Wno-unknown-warning-option -Wno-return-type-c-linkage -Wno-unused-function -Wno-sign-compare -Wno-shorten-64-to-32 -Wno-unused-local-typedefs -Wno-unused-but-set-parameter -Wno-unknown-attributes -Wno-null-pointer-subtraction)
set(WARNINGS ${WARNINGS} ${OSX_SUPPRESSIONS})

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -stdlib=libc++ -Wno-return-type-c-linkage -Wno-unneeded-internal-declaration")
Expand Down
4 changes: 3 additions & 1 deletion Release/include/cpprest/details/cpprest_compat.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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


//#define dllimport

#define novtable /* no novtable equivalent */
#define __assume(x) \
do \
Expand Down