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

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added inline comments.


================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:88-92
+  if (!AT->getElementType()->isCharType() &&
+      !AT->getElementType()->isChar8Type())
+    return false;
+
+  return true;
----------------
aaron.ballman wrote:
> Allowing `char8_t` seems wrong to me -- https://godbolt.org/z/zK74hMP7q
Just agreeing with Aaron there.
I don't have a problem  with making `__builtin_strcmp` work with any charN_t but in this case we need to make sure both parameters are of the same element type.
Your implementation seems to allow `__builtin_strcmp(u8"", "")` which is comparing very different things. ie `__builtin_strcmp(u8"a", "z")` could be true, which is really not something we want to support :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156212