dropbox / json11

A tiny JSON library for C++11.
MIT License
2.55k stars 613 forks source link

Use CXX_STANDARD to specify c++11 compile flags #71

Closed admsyn closed 8 years ago

admsyn commented 8 years ago

Uses CXX_STANDARD to specify a C++11 build, which avoids issues when a build contains C files.

Fixes #70

The tests seem to fail on my laptop, with or without this change (same output)

build ▹ ./json11_test
k1: v1
k3: ["a", 123, true, false, null]
    - "a"
    - 123
    - true
    - false
    - null
Result: {"a": 1, "b": "text", "c": [1, 2, 3]}
Result: {}
Failed: malformed comment
Failed: unexpected end of input inside inline comment
Failed: unexpected end of input inside comment
Failed: unexpected end of input inside multi-line comment
obj: {"k1": "v1", "k2": 42, "k3": ["a", 123, true, false, null]}
Assertion failed: (nested_array.array_items().size() == 1), function json11_test, file /Users/adam/workspace/json11/test.cpp, line 198.
Abort trap: 6
smarx commented 8 years ago

Automated message from Dropbox CLA bot

@admsyn, thanks for the pull request! It looks like you haven't yet signed the Dropbox CLA. Please sign it here.

admsyn commented 8 years ago

I have done so

artwyman commented 8 years ago

@admsyn I'm not seeing your CLA agreement show up in the back-end. Might've been a glitch with the web form. Can you try again? If all goes well, the CLA bot should ping this thread acknowledging it.

artwyman commented 8 years ago

These changes seem fine, though I don't use cmake so I haven't verified them. I can merge once the CLA-bot issue is resolved.

Note regarding the tests, the failure you're seeing is an intentional "canary" for a compiler issue which came up a year or so ago, and has since turned into a defect report with the standards committee. You can disable it when compiling with -DJSON11_ENABLE_DR1467_CANARY=0. I added the ability to do so from the command line with make, but didn't do so for cmake since I don't use it. If you want to add that ability to CMake too, I'd welcome that PR.

admsyn commented 8 years ago

Did the CLA again, double checked that I was using the same email as my github account. ¯_(ツ)_/¯

.. If you want to add that ability to CMake too, I'd welcome that PR.

Sure, enabled (=1) by default?

artwyman commented 8 years ago

Yes, enabled by default is the current strategy.

Ah, it looks like you signed the company CLA instead of the individual, which I hadn't dealt with before, and doesn't trigger the CLA bot. I've figured out things with our open-source team here so you're all set. Sorry for the delay.