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.

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

Compiling 3.6.0 with GCC > 7, array vs std::array #590 is back #1530

Closed
niklas88 opened this issue Mar 20, 2019 · 11 comments
Closed

Compiling 3.6.0 with GCC > 7, array vs std::array #590 is back #1530

niklas88 opened this issue Mar 20, 2019 · 11 comments
Assignees
Labels
kind: bug release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Milestone

Comments

@niklas88
Copy link

As a user I can't reopen an issue and I fear commenting on it might get lost.
Anyway as of the 3.6.0 release #590 is back from the dead.
My guess is that the fix of adding parenthesis was removed by some code formatter
here is the problematic line in the current version:

https://github.com/nlohmann/json/blob/release/3.6.0/include/nlohmann/json.hpp#L5731

Also this problem still appears with GCC 8.2.1

@nlohmann nlohmann self-assigned this Mar 20, 2019
nlohmann added a commit that referenced this issue Mar 20, 2019
@nlohmann nlohmann added the solution: proposed fix a fix for the issue has been proposed and waits for confirmation label Mar 20, 2019
@nlohmann
Copy link
Owner

Thanks for reporting! I am using GCC 7 and 8 in the CI, but did not catch this issue. Strange... Anyway, could you please check whether the issue is fixed in c790b9f?

@niklas88
Copy link
Author

Closing as the original issue has been reopened

@nlohmann nlohmann reopened this Mar 20, 2019
@nlohmann
Copy link
Owner

Let's have the regression here.

@niklas88
Copy link
Author

Ah, I'm too fast. Yes that patch should fix it as locally adding the parenthesis does

@nlohmann
Copy link
Owner


🔖 Release item

This issue/PR will be part of the next release of the library. This template helps preparing the release notes.

Type

  • ✨ New Feature
  • 🐛 Bug Fix
  • ⚡️ Improvement
  • 🔨 Further Change
  • 🔥 Deprecated function

Description


@niklas88
Copy link
Author

@nlohmann I just found out that the same code in the single_include/nlohmann/json.hpp still has this problem

@nlohmann
Copy link
Owner

I changed both files. What is the exact error message?

@nlohmann nlohmann reopened this Mar 20, 2019
@niklas88
Copy link
Author

The parenthesis are wrong it needs to be return (*lhs.m_value.array) < (*rhs.m_value.array); I guess I didn't look close enough in the commit

nlohmann added a commit that referenced this issue Mar 20, 2019
@nlohmann
Copy link
Owner

Could you try again with b33093d?

@niklas88
Copy link
Author

I've tested with the single include from b33093d and can confirm that it now compiles and works. I'm sorry for getting it wrong last time I think I just assumed that it was the same parenthesis as in 1a9d766 where I looked up the fix.

@nlohmann
Copy link
Owner

No worries - it was me who got this regression slipped in. Thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug release item: 🐛 bug fix solution: proposed fix a fix for the issue has been proposed and waits for confirmation
Projects
None yet
Development

No branches or pull requests

2 participants