search

vincentlaucsb / csv-parser

A high-performance, fully-featured CSV parser and serializer for modern C++.
MIT License
901 stars 150 forks source link

fix CONSTEXPR for C++14 and GNUC #165

Closed Jibbow closed 3 years ago

Jibbow commented 3 years ago

Issue: when using gcc >7.2 together with CSV_CXX_STANDARD 11/14, it would lead to CONSTEXPR evaluating to constexpr even though C++17 is not used. This was due to a logic error when handling a bug in a specific version of gcc.

This issue only appears when using gcc. It works when using clang.

vincentlaucsb commented 3 years ago

constexpr is not a C++17 feature, and has existed since C++11 (https://en.cppreference.com/w/cpp/language/constexpr). C++17 simply expands the cases where constexpr can be used.

The comment and linked article explain exactly why that condition is there: g++ prior to version 7.2 does not handle constexpr correctly in certain situations. Unless g++ >= 7.2 is producing an error when CONSTEXPR expands to constexpr, then there is nothing to fix.

Jibbow commented 3 years ago

Okay yes, that makes sense. However, I struggle to compile csv-parser with GCC and C++14 and I thought it has something to do with constexpr.

One of the errors was 

include/internal/data_type.h:224:9: error: the value of ‘csv::internals::CSV_INT8_MAX’ is not usable in a constant expression

It compiles fine with the changes in the PR, though.

Maybe there is something else going wrong? E.g. why does the macro expand to inline for "clang && !C++17" but for "gcc && !C++17" it would expand to constexpr (assuming that we don't have a gcc version with the bug). Wouldn't it make more sense to expand to the same thing for both compilers for the same version of C++?