jbattini / fast-cpp-csv-parser

Automatically exported from code.google.com/p/fast-cpp-csv-parser
0 stars 0 forks source link

Cannot build on windows. patch provided #6

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
What steps will reproduce the problem?
1. Build on windows with Visual Studio 2013

What is the expected output? What do you see instead?
Compile is successful

What version of the product are you using? On what operating system?
latest Windows

Please provide any additional information below.
1. Had to create a custom snprintf function since VS2013 still doesn't have one

2. Had to remove all the constexpr since VS2013 complains about those

3. Also VS2013 complains about multiple default constructors, this time it's 
seems right, these can either be chosen when no arguments are supplied
CSVReader() = delete;
explicit CSVReader(Args...args):in(std::forward<Args>(args)...){

I commented out the first one

Attached is the new file with all the patches

Original issue reported on code.google.com by gcflym...@gmail.com on 1 Aug 2014 at 2:40

Attachments:

GoogleCodeExporter commented 8 years ago
I had to do the same, which I did independently.
Would be great if the author made the package VS2013 compatible.

Original comment by gara...@gmail.com on 22 Sep 2014 at 6:43

GoogleCodeExporter commented 8 years ago
Issues 2 & 3 could be solved by adjusting code. However, I'm not going to patch 
without issue 1 also being resolved. There is no point in making it 
semi-compatible. Everything or nothing.

Issue 1 is very problematic. I do not want to add a function named snprintf out 
of fear of having problems with other compilers. I do not want a complete 
independent function named my_snprintf because I want to use the 
compiler/stdlib provided function if it is available. I do not want "#ifdef 
this_compiler" because I do not want to keep track of what compiler has which 
features.

Further the patch has some severe problems: "#define snprintf std::snprintf" is 
a nogo as it does not play nice with namespaces. Suppose some other code in an 
unrelated header uses "std::snprintf". The macro would turn this into 
"std::std::snprintf" and you get some obscure error messages pointing to 
completely unrelated lines of code. This is very bad. Another problem is that a 
newer version of VS might support "snprintf" and then you "#ifdef _MSC_VER" 
breaks.

I added a link beside the download link to this issue. This way people using VS 
will not keep discovering the same issue but know from the beginning what needs 
to be done.

> 3. Also VS2013 complains about multiple default constructors, 
> this time it's seems right, these can either be chosen when 
> no arguments are supplied

Actually I don't think VS is correct. CSVReader() = delete means that no 
default constructor should be generated, i.e., the code specifically states 
that there should be only one constructor. Obviously VS2013 generates some form 
of default constructor as otherwise it would not be ambiguous.

Original comment by strasser...@gmail.com on 28 Sep 2014 at 1:32

GoogleCodeExporter commented 8 years ago
An other fix suggestion for VS2013, IMHO less intruisive than the one suggested 
by the OP, but that defines a new function in the std namespace (and that is 
considered a bad practice). If std::snprintf is available by other means then 
define CSV_IO_MSVC_NO_SNPRINTF.

#if defined(_MSC_VER) && (_MSC_VER <= 1800)

#if !defined(constexpr)
#   define constexpr
#endif

#if !defined(CSV_IO_MSVC_NO_SNPRINTF)

    namespace std {

        inline int c99_vsnprintf(char* str, size_t size, const char* format, va_list ap)
        {
            int count = -1;

            if (size != 0)
                count = _vsnprintf_s(str, size, _TRUNCATE, format, ap);
            if (count == -1)
                count = _vscprintf(format, ap);

            return count;
        }

        inline int snprintf(char* str, size_t size, const char* format, ...)
        {
            int count;
            va_list ap;

            va_start(ap, format);
            count = c99_vsnprintf(str, size, format, ap);
            va_end(ap);

            return count;
        }

        using namespace std;

    } //namespace std

#endif //CSV_IO_MSVC_NO_SNPRINTF

#endif // _MSC_VER

Original comment by samuel.d...@gmail.com on 14 Jan 2015 at 10:48