aaron.ballman added inline comments.
================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:88
+ else if (StringRef(Str).getAsInteger(0, Fill))
+ return false;
+
----------------
Please add test coverage for a case like `__builtin_nan("derp")` to show it's
not a valid constant expression.
================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:106
+ // 2008 revisions, MIPS interpreted sNaN-2008 as qNan and qNaN-2008 as
+ // sNaN. This is now known as "legacy NaN" encoding.
+ if (Signaling)
----------------
LOL, thank you for this comment and wow. :-D
================
Comment at: clang/lib/AST/Interp/InterpBuiltin.cpp:135-137
+ case Builtin::BI__builtin_nanl:
+ case Builtin::BI__builtin_nanf16:
+ case Builtin::BI__builtin_nanf128:
----------------
We should have test coverage for these odd sizes just to ensure we're properly
pushing/popping what we expect off the stack.
================
Comment at: clang/test/AST/Interp/builtin-functions.cpp:44
+
+ constexpr float Nan2 = __builtin_nans([](){return "0xAE98";}()); //
ref-error {{must be initialized by a constant expression}}
+}
----------------
Probably worth a comment here mentioning that the ref-error is a rejects-valid
issue that the experimental compiler accepts correctly.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155356/new/
https://reviews.llvm.org/D155356
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
