[PATCH] D135750: [clang][Interp] Track initialization state of local variables

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:835
     IsTemporary = true;
     Ty = E->getType();
   }
----------------
Do we really want to the type of the expression? If we have a `ValueDecl` don't we want that type? I think they should be the same, do you have any examples where the expression is the type we want? I am looking at the AST from ` int x = 1+1L;` 

https://godbolt.org/z/q3Tr7Kxoq


================
Comment at: clang/lib/AST/Interp/Descriptor.h:73
+  /// Flag indicating if the field is mutable (if in a record).
+  unsigned IsMutable : 1;
+
----------------
Maybe `IsFieldMutable` b/c we call `CreateDescriptor` it is a little confusing why we have `IsConst` and `IsMutable`


================
Comment at: clang/lib/AST/Interp/Pointer.h:63-64
 private:
   static constexpr unsigned PastEndMark = (unsigned)-1;
   static constexpr unsigned RootPtrMark = (unsigned)-1;
 
----------------
or `-1u`  


================
Comment at: clang/test/AST/Interp/cxx20.cpp:65
 }
-static_assert(unInitLocal() == 0, ""); // expected-error {{not an integral constant expression}} \
-                                       // ref-error {{not an integral constant expression}} \
-                                       // ref-note {{in call to 'unInitLocal()'}}
-
-/// TODO: The example above is correctly rejected by the new constexpr
-///   interpreter, but for the wrong reasons. We don't reject it because
-///   it is an uninitialized read, we reject it simply because
-///   the local variable does not have an initializer.
-///
-///   The code below should be accepted but is also being rejected
-///   right now.
-#if 0
+static_assert(unInitLocal() == 0, ""); // ref-error {{not an integral constant expression}} \
+                                       // ref-note {{in call to 'unInitLocal()'}} \
----------------
Can we also test local arrays and classes as well?


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

https://reviews.llvm.org/D135750