diffblue / cbmc

C Bounded Model Checker
https://diffblue.github.io/cbmc
Other
848 stars 265 forks source link

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

Open karkhaz opened 6 years ago

karkhaz commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago
/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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

karkhaz commented 6 years ago

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 commented 6 years ago

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

karkhaz commented 6 years ago

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 commented 6 years ago

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

karkhaz commented 6 years ago

Cool! I don't think this is super-urgent, which is good because most of the Clang developers are probably still sleeping :slightly_smiling_face: 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 commented 6 years ago

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 commented 6 years ago

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

smowton commented 6 years ago

@karkhaz worth pinging them?

karkhaz commented 6 years ago

@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 commented 6 years ago

@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.