Enable long long by default (needed by filezilla) about pugixml HOT 24 CLOSED

Hmm, this is tricky. The reason the condition is worded in that fashion is because there are some platforms (that of course are an exception rather than the rule) that could have recent GCC but no support for long long. I don't have examples OTOH but I had these kinds of issues in the past.

I currently take the short cut to say that on MSVC the runtime library always has adequate support since some version, and on other compilers you have to have C++0x/11 mode enabled. Is it possible that the solution is to enable C++11 mode when packaging by default?

Alternatively I could say that if your GCC is relatively recent the hope is that your runtime library is C99 compliant and as such has support for long long. Not sure what the version cutoff would be - I'll check the versions I had issues with in the past.

from pugixml.

I don't think it's true that there are recent versions of GCC that don't support long long however there are language dialects that don't support it, even with the very latest version e.g.

$ echo "long long i = 0LL;"  | g++ -std=c++03 -pedantic-errors -c -x c++ -
<stdin>:1:15: error: use of C++11 long long integer constant [-Wlong-long]
<stdin>:1:6: error: ISO C++ 1998 does not support ‘long long’ [-Wlong-long]

And even if the compiler supports it, that says nothing about whether libc supports it. What library features (such as strtoll) are needed?

from pugixml.

pugixml only needs strtoll and %lld specifier for printf. I can implement them myself though, which would leave a question of having the type itself. If there is some way (GCC-specific is fine) to detect support of the type itself I can stop relying on C++11 I guess.

I was thinking of checking if LLONG_MAX is defined; not sure how reliable that is. Maybe there's a better way. I'll look into this.

I did not have issues with long long specifically; I had issues with something else where I used a GCC version check to check for some std library feature and then there was a platform with recent GCC (well, this was ~5 years ago...) and weird standard library. Which is why I originally played it safe and used C++11 as a toggle but this can be changed.

from pugixml.

Ugh, -pedantic is annoying.

I'm reasonably sure just checking for long long type support is good enough - e.g. something like:

#   elif defined(__GNUC__) && defined(__SIZEOF_LONG_LONG__) && __SIZEOF_LONG_LONG__ == 8 && !defined(__STRICT_ANSI__)

Slightly worried this may not work on some embedded platforms but there's no way to know that beforehand. However, -pedantic (which is used by pugixml test suite...) seems impossible to work around since there's no define that changes when you enable it?

from pugixml.

Hi,

I don't think it's reliable to evaluate with cases the availability of long long.
At least cmake seems to have a Module to test it at buildtime: (like autotool already has).
cmake/Modules/CheckCXX11Features.cmake (not yet from cmake upstream)

So I don't see any salvation except to test at buildtime. This shoudn't be a new question for pugixml alone. Every package should be doing so.

Thx

from pugixml.

Slightly worried this may not work on some embedded platforms

I don't see why it wouldn't work. The C99 and C++11 standards require long long but they don't require it to be larger than long so the worst case scenario would be that GCC defines it to be the same size as long, but it would still define it.

Don't you also want to check either __STDC_VERSION__ or __cplusplus to check the language standard in use? Otherwise -std=c++11 -pedantic would fail your check, even though it is required to support long long.

from pugixml.

@jwakely C++11 is checked as a first thing; if it is supported I just assume long long is present and fully functional, including library functions.

I know the standard guarantees but they do not necessarily hold. I've seen platforms with wchar_t type being present but no standard (C++03!) wchar_t functions being available. I would not be surprised to see a platform that advertises C99 support but %lld support is accidentally compiled out of printf code or long long codegen generates an ICE. Regardless, it's impossible to check for C99 support from C++ code... which is why originally I enabled long long only for C++11 or MSVC 2008+ where I trust the CRT more.

from pugixml.

C++11 is checked as a first thing; if it is supported I just assume long long is present and fully functional, including library functions.

Ah great.

I would not be surprised to see a platform that advertises C99 support but %lld support is accidentally compiled out of printf code or long long codegen generates an ICE.

Yes, that's true. Sorry, I forgot that you'd already told me you need library support not just compiler support. There are internal macros defined by libstdc++ that indicate whether proper C99 library support for long long is present, but they are not meant to be used by user code. Libstdc++ probes the C library to determine that support while building GCC.

from pugixml.

cmake >= 3.1 has commands to check for compiler features, with something like:

target_compile_features(pugixml PUBLIC cxx_long_long_type)

compilation was done automatically with c++11 activated

(checking how we could enable long long support in Gentoo package, also for filezilla)

from pugixml.

Can you clarify this?

compilation was done automatically with c++11 activated

If pugixml is compiled with C++11 it should get PUGIXML_HAS_LONG_LONG automatically set as per https://github.com/zeux/pugixml/blob/master/src/pugixml.hpp#L67-L73. I'm not sure if doing this changes ABI (it may switch standard library?) but generally this is probably the best way to solve this...

from pugixml.

I'm not sure if doing this changes ABI (it may switch standard library?)

No, there is only one libstdc++.so not one per set of compiler options.

from pugixml.

And indeed, the directive tells cmake to check for a C++ compiler that supports long long types, which (just retested), makes it use "-std=gnu++11" here. And this in turn sets __cplusplus correctly for pugixml.hpp check.

But just adding this line would make it mandatory, this can probably be wrapped in a configuration option?

from pugixml.

No, there is only one libstdc++.so not one per set of compiler options.

Ok, that's good to know (there are two libraries on OSX). However, even with one library, is it correct to assume that ABI for functions that use std::string/std::istream/std::ostream in the interface (there are some in pugixml) changes when switching the library? Is it both streams & strings or just strings?

from pugixml.

I don't know what "switching the library" refers to if you only have one library.

If two versions of libstdc++.so are built with different settings for any options that affect the library ABI, then they have different ABIs. If the two versions are built with the same settings for options that affect ABI, they have the same ABI. https://gcc.gnu.org/onlinedocs/libstdc++/manual/configure.html documents which options change the ABI.

If you're only talking about switching from one build of libstdc++.so to another, I don't think there are any configurations that alter the ABI of iostream classes, but potions such as --enable-fully-dynamic-string alter the ABI for std::string.

If you're talking about switching from e.g. libstdc++.so to libc++.so then nearly all types (except exception classes) are completely incompatible between the two.

from pugixml.

Sorry, I meant switching configuration options. I'm worried that enabling C++11 for pugixml will make libpugixml1.so incompatible with the packages that use it (because of std:: types in some functions in the interface).

from pugixml.

For libstdc++ (and I believe libc++) using either C++03 or C++11 makes no difference in terms of ABI.

There are no types which have one definition in C++03 and a different definition in C++11. That would prevent mixing code compiled with different -std options, which would be a Bad Thing.

However, for libstdc++ prior to GCC5 the C++11-specific types were not yet stable and so e.g. std::tuple is different in GCC 4.9 and GCC 5. But that's a difference between compiler versions, not -std modes.

from pugixml.

Or put another way: using -std=c++11 in your library or your application does not affect the ABI of libstdc++.so, because you're not reconfiguring and rebuilding GCC and libstdc++.so, you're still just using them.

from pugixml.

Ok, that makes sense. Thanks! I was assuming that ABI changes because I thought that std::string implementation has to be revised in C++11 to remove COW but apparently this only happened in GCC5 (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=21334#c47).

In that case, it should be safe to automatically enable C++11 in CMakeLists.txt if the compiler supports it.

from pugixml.

It happened in GCC 5 but it is independent of the -std option. Whether you compile with old or new strings is the same whether you use -std=c++03 or -std=c++11

from pugixml.

So the real action item here is enabling C++11 support in CMake. I would prefer if this did not require CMake 3.1 but otherwise the change feels safe.

from pugixml.

You could just use add_compile_flags("-Wno-long-long") on older CMake with g++. (Try echo "long long i = 0LL;" | g++ -std=c++03 -pedantic-errors -Wno-long-long -c -x c++ -, it compiles here.)

(By the way, at least C99 requires long long to be at least 64 bits wide. long is only required to be at least 32 bits, though they can both be 64 bits wide, as on x86_64 GNU/Linux. A target that makes long long smaller than 64 bits is not compliant.)

from pugixml.

Hello.

Is set cxx_long_long_type at target_compile_features at pugixml prevent to use std set to c++14 at programs that use library?

Please view a quick sample:
CMakeLists.txt

cmake_minimum_required(VERSION 3.3)

project("check_pugixml")

set(EXTERNAL_PATH ${CMAKE_CURRENT_SOURCE_DIR}/third-party)
set(EXTERNAL_INCLUDES)
set(EXTERNAL_LIBRARIES)

set(EXTERNAL_PATH_PUGIXML ${EXTERNAL_PATH}/pugixml)

add_subdirectory(${EXTERNAL_PATH_PUGIXML})

# target_compile_options(pugixml PUBLIC $<$<COMPILE_LANGUAGE:CXX>:-std=c++14>)
set_target_properties(pugixml PROPERTIES POSITION_INDEPENDENT_CODE ON)
target_compile_definitions(pugixml PRIVATE PUGIXML_NO_EXCEPTIONS)
list(APPEND EXTERNAL_INCLUDES ${EXTERNAL_PATH_PUGIXML}/src)
list(APPEND EXTERNAL_LIBRARIES pugixml)

add_executable(demo ${CMAKE_CURRENT_SOURCE_DIR}/main.cpp)
target_link_libraries(demo ${EXTERNAL_LIBRARIES})
target_include_directories(demo PRIVATE ${EXTERNAL_INCLUDES})
target_compile_options(demo PUBLIC $<$<COMPILE_LANGUAGE:CXX>:-std=c++14>)
target_compile_options(demo PUBLIC $<$<CONFIG:DEBUG>:-Wall -Wextra -Werror>)
target_compile_options(demo PUBLIC $<$<CONFIG:RELEASE>:-O3 -ffast-math -Wall -Wextra -Werror>)
set_target_properties(demo PROPERTIES POSITION_INDEPENDENT_CODE ON)

main.cpp

#include <pugixml.hpp>
#include <sstream>
#include <iostream>

int main(int argc, char** argv)
{
    //std::cout << __cplusplus << std::endl;
    //
    std::ostringstream s;
    s << "<?xml version=\"1.0\" encoding=\"UTF-8\"?><cplusplus version=\"" << __cplusplus << "\"></cplusplus>";
    auto data = s.str();
    //
    pugi::xml_document doc;

    if(!doc.load_buffer_inplace_own(&data.front(), data.size()))
    {
        return -1;
    }

    std::cout << doc.child("cplusplus").attribute("version").as_string() << std::endl;

    return 0;
}

What will be outputted? And what if we remove target_compile_features(pugixml PUBLIC cxx_long_long_type) from pugixml CMakeLists.txt?
CMake add std set to gnu++11 to target from dependence pugixml that already set to c++14 - just check flags.make files in CMAKE_BINARY_DIRECTORY

Personally I resolve that situation by own add_library of pugixml

set(SOURCES
    ${EXTERNAL_PATH_PUGIXML}/src/pugixml.hpp
    ${EXTERNAL_PATH_PUGIXML}/src/pugiconfig.hpp
    ${EXTERNAL_PATH_PUGIXML}/src/pugixml.cpp)
add_library(pugixml STATIC ${SOURCES})
target_compile_options(pugixml PUBLIC $<$<COMPILE_LANGUAGE:CXX>:-std=c++14>)
set_target_properties(pugixml PROPERTIES POSITION_INDEPENDENT_CODE ON)
list(APPEND EXTERNAL_INCLUDES ${EXTERNAL_PATH_PUGIXML}/src)
list(APPEND EXTERNAL_LIBRARIES pugixml)

But maybe that situation can be resolve on library side so add_subdirectory can be used in programs that use pugixml library.

Thank you.

from pugixml.

@TheVice My understanding is that this happens because CMake does not recognize your -std=c++14 option as setting the C++ standard, so the flags.make ends up with both c++14 and gnu++11.

As far as I can tell, replacing manual specification of the compile flag with this:

set_target_properties(demo PROPERTIES CXX_STANDARD 14)

in your CMakeLists.txt fixes the problem - CMake correctly leaves just the option that is responsible for enabling C++14 support.

from pugixml.

@zeux thank for your response. Your advice resolve my question so I can use add_subdirectory.

from pugixml.