[PATCH] D156212: [clang][Interp] Implement remaining strcmp builtins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.


================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:88-92
+  if (!AT->getElementType()->isCharType() &&
+      !AT->getElementType()->isChar8Type())
+    return false;
+
+  return true;
----------------
Allowing `char8_t` seems wrong to me -- https://godbolt.org/z/zK74hMP7q


================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:147-149
+  // FIXME: Implement support for the wide versions.
+  if (IsWide)
+    return false;
----------------
Let's either implement support for them in this patch or exclude the wchar_t changes entirely from the patch?


================
Comment at: clang/test/AST/Interp/builtin-functions.cpp:225-229
+#if 0
+constexpr int a = strcmp("hello", "world"); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strcmp' cannot be used in a constant expression}}
+  constexpr int b = strncmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strncmp' cannot be used in a constant expression}}
+  constexpr int c = memcmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'memcmp' cannot be used in a constant expression}}
+#endif
----------------
Why are these in a `#if 0` block?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156212/new/

https://reviews.llvm.org/D156212