[PATCH] D131942: [clang][Inter[ Implement bool and nullptr literal expressions

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: clang/lib/AST/Interp/Disasm.cpp:26
 inline std::enable_if_t<!std::is_pointer<T>::value, T> ReadArg(Program &P,
-                                                               CodePtr OpPC) {
+                                                               CodePtr &OpPC) {
   return OpPC.read<T>();
----------------
Are you sure this isn't intentional that this is passed by value?  With a name like CodePtr, it certainly SOUNDS like it means to be passed by value.


================
Comment at: clang/lib/AST/Interp/Function.h:69
   /// Returns a pointer to the start of the code.
-  CodePtr getCodeBegin() const;
+  CodePtr getCodeBegin() const { return Code.data(); }
   /// Returns a pointer to the end of the code.
----------------
is this an unrelated NFC change?  In the future, please keep this out of patches that do stuff.


================
Comment at: clang/lib/AST/Interp/Source.h:64
   /// Pointer into the code owned by a function.
   const char *Ptr;
 };
----------------
Ah, this DOES just wrap a pointer, passing by value is likely the right idea here.


================
Comment at: clang/test/AST/Interp/literals.cpp:14
+constexpr bool getTrue() { return true; }
+constexpr bool getFalse() { return false; }
----------------
can you also do a 'getNullptr'?  Also, a static-assert for all 3 of the functions as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131942