[PATCH] D155369: [clang][Interp] Implement __builtin_isnan()

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.


================
Comment at: clang/lib/AST/Interp/Function.h:180
+    // FIXME: Is there a better way to get this information?
+    if (getName() == "__builtin_isnan")
+      return true;
----------------
cor3ntin wrote:
> You can compare getBuiltinID(), which is better than using strings, it's what we seem to be doing elsewhere
Yeah but that still sucks. Is there no way to query whether the builtin has `t` set ("signature is meaningless, use custom typechecking"), regardless of the ID or name?


================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:169
+  // refactor this into a different function that checks things are valid.
+  const Floating &Arg = S.Stk.peek<Floating>();
+
----------------
cor3ntin wrote:
> There are only a few functions created by SemaBuiltinFPClassification - only two of which are not unary. this looks fine, fpclassify and isfpclass can have their own logic.
> I don't suppose there is a way to assert we have one argument there?
We could (and maybe want) to pass along the `CallExpr`, but the arguments and their number are already checked in `Sema`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D155369