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

Add the [[nodiscard]] attribute to the result class #72

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

indiosmo
Copy link

@indiosmo indiosmo commented May 9, 2024

No description provided.

@zajo
Copy link
Collaborator

zajo commented May 9, 2024

The difficulty with these failures is that with LEAF being a standalone library, it doesn't use Boost Config. We need the correct compiler-specific ifdefs in leaf/config.hpp.

@indiosmo
Copy link
Author

indiosmo commented May 9, 2024

Not sure I understand, the macro in question is already defined in leaf/config.hpp, line 212.

  #if defined(__has_attribute) && defined(__SUNPRO_CC) && (__SUNPRO_CC > 0x5130)
  #   if __has_attribute(nodiscard)
  #       define BOOST_LEAF_ATTRIBUTE_NODISCARD [[nodiscard]]
  #   endif
  #elif defined(__has_cpp_attribute)
      //clang-6 accepts [[nodiscard]] with -std=c++14, but warns about it -pedantic
  #   if __has_cpp_attribute(nodiscard) && !(defined(__clang__) && (__cplusplus < 201703L)) && !(defined(__GNUC__) && (__cplusplus < 201100))
  #       define BOOST_LEAF_ATTRIBUTE_NODISCARD [[nodiscard]]
  #   endif
  #endif
  #ifndef BOOST_LEAF_ATTRIBUTE_NODISCARD
  #   define BOOST_LEAF_ATTRIBUTE_NODISCARD
  #endif
  • edit1
    Perhaps you mean this is not sufficient, and boost config handles the missing cases, is that it?

  • edit2
    I'm looking into Boost Config and the only mention of [[nodiscard]] is in detail/suffix.hpp, and it's exactly the same macro definition that is in leaf/config.hpp so I'm not sure how Boost Config would help here.

@zajo
Copy link
Collaborator

zajo commented May 10, 2024

I mean the ifdefs are not correct, we need to inspect the error logs to see what's the problem. The goal is to make leaf/config.hpp behave the same as boost/config.hpp (which I presume is correct) in terms of detecting the relevant [nodiscard] support in various compiler versions. I might be able look into it during the weekend.

@indiosmo
Copy link
Author

indiosmo commented May 10, 2024

I looked into boost/config.hpp and it seems to be exactly the same as leaf/config.hpp as far as [[nodiscard]] is concerned.

Also, I tried looking at the CI logs but all the ones that failed are blank after line 49.

image

@zajo
Copy link
Collaborator

zajo commented May 10, 2024

The logs don't end at line 49, it just takes a while for github to fetch the whole thing. The logs are tens of thousands of lines long, that's why. :)

@indiosmo
Copy link
Author

indiosmo commented May 10, 2024

I've reverted the changes to see if all tests pass, as at a cursory look it seems that some of the errors are unrelated to the change. Let's see.

edit:
Nevermind, here's a failure for nodiscard specifically:

2024-05-10T16:59:48.1125656Z In file included from ./boost/leaf/result.hpp:9:0,
2024-05-10T16:59:48.1128442Z from libs/leaf/test/_hpp_result_test.cpp:6:
2024-05-10T16:59:48.1129879Z ./boost/leaf/config.hpp:219:47: error: expected identifier before '[' token
2024-05-10T16:59:48.1131442Z # define BOOST_LEAF_ATTRIBUTE_NODISCARD [[nodiscard]]

@zajo
Copy link
Collaborator

zajo commented May 11, 2024

Yes, the issue is in the nodiscard macro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants