Skip to content

C++2a <=> operator breaks the build #1974

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

Open
karkhaz opened this issue Mar 26, 2018 · 19 comments
Open

C++2a <=> operator breaks the build #1974

karkhaz opened this issue Mar 26, 2018 · 19 comments

Comments

@karkhaz
Copy link
Collaborator

karkhaz commented Mar 26, 2018

One last build failure when using a recent compiler (Clang 6.0 with experimental C++2a support):

/cbmc/src/util/irep_ids.def:44:18: error: '<=>' is a single token in C++2a; add a space to avoid a change in behavior [-Werror,-Wc++2a-compat]
IREP_ID_TWO(iff, <=>)
                 ^

<=> is the "spaceship operator" that might be introduced in C++ at some point. This is a bit weird because the <=> is supposed to be stringified by the macro, but Clang still sees it for some reason. Anyhow, I haven't patched this in #1973, but it can be suppressed for now by building with -Wno-c++2a-compat.

@tautschnig
Copy link
Collaborator

I'm wondering whether this some kind of bug in the way Clang generates those warnings? I'd be curious whether it does the same for

#include <cstring>
#include <cassert>

#define op <=>
const char *s = #op;
const char *s2 = "<=>";

int main(int argc, char *argv[])
{
  assert(strcmp(s, s2) == 0);
  return 0;
} 

? I'd suggest to add a pragma to override this, possibly just for Clang - take a look at various util/pragma*.

@karkhaz
Copy link
Collaborator Author

karkhaz commented Mar 26, 2018

/tmp/foo.cpp:4:12: warning: '<=>' is a single token in C++2a; add a space to avoid a
      change in behavior [-Wc++2a-compat]
#define op <=>
           ^

/tmp/foo.cpp:5:17: error: expected expression
const char *s = #op;
                ^
1 warning and 1 error generated.

Line 4 is #define op <=>. Yeah, maybe a bug? This seems a bit silly. I'll ask about it on the mailing list, and investigate adding a pragma meanwhile.

@tautschnig
Copy link
Collaborator

Yeah, maybe a bug? This seems a bit silly. I'll ask about it on the mailing list, and investigate adding a pragma meanwhile.

Brilliant, thanks!

@karkhaz
Copy link
Collaborator Author

karkhaz commented Mar 26, 2018

BTW should I copy you in? If not, I can just post a summary over here when I've heard back from clang developers.

@tautschnig
Copy link
Collaborator

BTW should I copy you in?

Happy to be copied in, but also please do post a summary here or a link to the thread if there is a public archive (I suppose so).

@smowton
Copy link
Contributor

smowton commented Mar 26, 2018

@karkhaz what happens if you use clang++ -E to preprocess in isolation, then clang++ -c afterwards? Does an unquoted <=> appear in the clang++ -E output?

@karkhaz
Copy link
Collaborator Author

karkhaz commented Mar 26, 2018

I believe @tautschnig's example is slightly wrong, I don't think you can use the # character in arbitrary C code, it needs to be in a define. Here's a slightly different example:

  #include <cstring>
  #include <cassert>

  #define op <=>
  #define str(s) #s

  const char *s = str(op);
  const char *s2 = "<=>";

  int main(int argc, char *argv[])
  {
    assert(strcmp(s, s2) == 0);
    return 0;
  }

The preprocessed output of that is

const char *s = "op";
const char *s2 = "<=>";

int main(int argc, char *argv[])
{
  (static_cast <bool> (strcmp(s2, s2) == 0) ? void (0) : __assert_fail ("strcmp(s, s2) == 0", "foo.cpp", 12, __extension__ __PRETTY_FUNCTION__));
  return 0;
}

clang++ does not complain about this when I compile it! (But does emit the warning when I don't do the preprocessing step separately). Thanks for the suggestion, I'll mention it to the developers...

@karkhaz
Copy link
Collaborator Author

karkhaz commented Mar 26, 2018

Ah, nonsense. Sorry, here's the correct version:

#include <cstring>
#include <cassert>

#define op <=>
#define xstr(s) str(s)
#define str(s) #s

const char *s = xstr(op);
const char *s2 = "<=>";

int main(int argc, char *argv[])
{
  assert(strcmp(s, s2) == 0);
  return 0;
}

Preprocessed:

const char *s = "<=>";
const char *s2 = "<=>";

int main(int argc, char *argv[])
{
  (static_cast <bool> (strcmp(s, s2) == 0) ? void (0) : __assert_fail ("strcmp(s, s2) == 0", "foo.cpp", 13, __extension__ __PRETTY_FUNCTION__));
  return 0;
}

Clang complains both when I preprocess it with -E, and when compiling it directly. If I compile the preprocessed file, it obviously doesn't complain. If I compile it directly, the assertion still manages to pass, so it's compiling correctly but the warning is spurious.

Sorry for the confusion.

@tautschnig
Copy link
Collaborator

Why is it that the xstr step is necessary? I'm wondering, because this might need to be fixed in irep_ids.cpp.

@karkhaz
Copy link
Collaborator Author

karkhaz commented Mar 26, 2018

Because str(op) is preprocessed to "op", even if we previously had a #define op <=>. You need to pre-process twice if you want to stringify the expansion of op rather than the literal op itself. See the example at the bottom of the page on stringification.

@tautschnig
Copy link
Collaborator

So maybe that's ok, because we have IREP_ID_TWO in there. Might be worth checking, though.

@karkhaz
Copy link
Collaborator Author

karkhaz commented Mar 26, 2018

Not sure what you want me to check?

I believe irep_ids.cpp and irep_ids.def are both fine, the macro should expand to "<=>" wherever it's used. I think this does happen, since with -Wno-c++2a-compat the code compiles just fine...

@tautschnig
Copy link
Collaborator

Ok, thanks, then we're good. Let's see what the Clang people say.

@karkhaz
Copy link
Collaborator Author

karkhaz commented Mar 26, 2018

Cool! I don't think this is super-urgent, which is good because most of the Clang developers are probably still sleeping 🙂 The mail that I sent to the list needs to be moderated, not sure how long that will take.

In theory this issue only affects users of Arch Linux, Gentoo, etc., and also people who compile Clang from source. Even then, switching off the warning should be fine until the Clang people clarify.

@karkhaz
Copy link
Collaborator Author

karkhaz commented Mar 26, 2018

Duncan Exon Smith confirms that this is likely a bug. I need to open a bugzilla account to file for this, but I'll update this thread if anything interesting happens.

https://lists.llvm.org/pipermail/cfe-dev/2018-March/057396.html

@karkhaz
Copy link
Collaborator Author

karkhaz commented Mar 28, 2018

Filed a bug here: https://bugs.llvm.org/show_bug.cgi?id=36925

@smowton
Copy link
Contributor

smowton commented Jun 20, 2018

@karkhaz worth pinging them?

smowton added a commit to smowton/cbmc that referenced this issue Jun 20, 2018
clang++-6.0 warns that in the next C++ standard <=> will be an operator and therefore an atomic
lexical token. I think that's overly zealous, but it's easily worked around by explicitly quoting
the name, which will be stringified before use in any case.

See diffblue#1974 for more details, or
https://bugs.llvm.org/show_bug.cgi?id=36925 for an upstream bug report.
@karkhaz
Copy link
Collaborator Author

karkhaz commented Jun 21, 2018

@smowton I'm going to ask for advice on IRC about fixing this. Basically the warning is being emitted from here, i.e. in the lexer. IMO, this is too early to decide whether the warning should be emitted, as you need some context to decide whether the use of <=> is inappropriate (an example of the sort of thing Clang wants to check for is this test case for spaceship abuse). But it's difficult for me to know where to insert the check, it seems complicated to do it in a context-sensitive way.

Anyway, I will ask for advice on IRC tonight, and if I get nothing satisfactory, I'll ping the bug report.

@karkhaz
Copy link
Collaborator Author

karkhaz commented Jun 21, 2018

@smowton however, if your commit fixes that problem, then I would recommend opening a PR for it---even if the Clang bug is fixed fairly soon, I'm not sure how quickly it would propagate to the distro versions of Clang. It would suck if Ubuntu or whatever started using Clang 6.0 and broke a lot of people's builds, it would be nice to preempt that.

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

No branches or pull requests

3 participants