[clang][Interp] Handle missing local initializers better

[tbaeder]

Summary

This is illegal in a constexpr context. We can already figure that out,
but we'd still run into an assertion later on when trying to visit the
missing initializer or run the invalid function.

[erichkeane]

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

245

Would be nice to test this before the work to allocate Offset?

clang/lib/AST/Interp/Function.h

163

[tbaeder]

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

245

Ah, sure. Good catch.

[shafik]

LGTM

clang/lib/AST/Interp/ByteCodeEmitter.cpp

Expected<Function *> ByteCodeEmitter::compileFunc(const FunctionDecl *F) {
   // Create a handle over the emitted code.
   Function *Func = P.createFunction(F, ParamOffset, std::move(ParamTypes),
                                     std::move(ParamDescriptors));
   // Compile the function body.
   if (!F->isConstexpr() || !visitFunc(F)) {
     // Return a dummy function if compilation failed.
     if (BailLocation)
       return llvm::make_error<ByteCodeGenError>(*BailLocation);
-    else
+    else {
+      Func->setIsFullyCompiled(true);
       return Func;
+    }
   } else {
     // Create scopes from descriptors.
     llvm::SmallVector<Scope, 2> Scopes;
     for (auto &DS : Descriptors) {
       Scopes.emplace_back(std::move(DS));
     }
 
     // Set the function's code.
     Func->setCode(NextLocalOffset, std::move(Code), std::move(SrcMap),
                   std::move(Scopes));
+    Func->setIsFullyCompiled(true);
     return Func;
   }
 }
 
 Scope::Local ByteCodeEmitter::createLocal(Descriptor *D) {
   NextLocalOffset += sizeof(Block);
   unsigned Location = NextLocalOffset;
   NextLocalOffset += align(D->getAllocSize());

clang/lib/AST/Interp/ByteCodeExprGen.cpp

if (const auto *FuncDecl = dyn_cast_or_null<FunctionDecl>(Callee)) {
     // Templated functions might not have been compiled yet, so do it now.
     if (!Func) {
       if (auto R =
               ByteCodeStmtGen<ByteCodeEmitter>(Ctx, P).compileFunc(FuncDecl))
         Func = *R;
     }
     assert(Func);
 
+    // If the function is being compiled right now, this is a recursive call.
+    // In that case, the function can't be valid yet, even though it will be
+    // later.
+    // If the function is already fully compiled but not constexpr, it was
+    // found to be faulty earlier on, so bail out.
+    if (Func->isFullyCompiled() && !Func->isConstexpr())
+      return false;
+
     QualType ReturnType = E->getCallReturnType(Ctx.getASTContext());
     Optional<PrimType> T = classify(ReturnType);
 
     if (T || ReturnType->isVoidType()) {
       // Put arguments on the stack.
       for (const auto *Arg : E->arguments()) {
         if (!this->visit(Arg))
           return false;

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

bool ByteCodeStmtGen<Emitter>::visitVarDecl(const VarDecl *VD) {
 
   if (!VD->hasLocalStorage()) {
     // No code generation required.
     return true;
   }
 
   // Integers, pointers, primitives.
   if (Optional<PrimType> T = this->classify(DT)) {
+    const Expr *Init = VD->getInit();
+
+    if (!Init)
+      return false;
+
     auto Off = this->allocateLocalPrimitive(VD, *T, DT.isConstQualified());
-    // Compile the initialiser in its own scope.
+    // Compile the initializer in its own scope.
     {
       ExprScope<Emitter> Scope(this);
-      if (!this->visit(VD->getInit()))
+      if (!this->visit(Init))
         return false;
     }
     // Set the value.
     return this->emitSetLocal(*T, Off, VD);
   } else {
     // Composite types - allocate storage and initialize it.
     if (auto Off = this->allocateLocal(VD)) {
       return this->visitLocalInitializer(VD->getInit(), *Off);

clang/lib/AST/Interp/Function.h

public:
   bool isConstexpr() const { return IsValid; }
 
   /// Checks if the function is virtual.
   bool isVirtual() const;
 
   /// Checks if the function is a constructor.
   bool isConstructor() const { return isa<CXXConstructorDecl>(F); }
 
+  /// Checks if the function is fully done compiling.
+  bool isFullyCompiled() const { return IsFullyCompiled; }
+
 private:
   /// Construct a function representing an actual function.
   Function(Program &P, const FunctionDecl *F, unsigned ArgSize,
            llvm::SmallVector<PrimType, 8> &&ParamTypes,
            llvm::DenseMap<unsigned, ParamDescriptor> &&Params);
 
   /// Sets the code of a function.
   void setCode(unsigned NewFrameSize, std::vector<char> &&NewCode, SourceMap &&NewSrcMap,
                llvm::SmallVector<Scope, 2> &&NewScopes) {
     FrameSize = NewFrameSize;
     Code = std::move(NewCode);
     SrcMap = std::move(NewSrcMap);
     Scopes = std::move(NewScopes);
     IsValid = true;
   }
 
+  void setIsFullyCompiled(bool FC) { IsFullyCompiled = FC; }
+
 private:
   friend class Program;
   friend class ByteCodeEmitter;
 
   /// Program reference.
   Program &P;
   /// Location of the executed code.
   SourceLocation Loc;
private:
   /// List of block descriptors.
   llvm::SmallVector<Scope, 2> Scopes;
   /// List of argument types.
   llvm::SmallVector<PrimType, 8> ParamTypes;
   /// Map from byte offset to parameter descriptor.
   llvm::DenseMap<unsigned, ParamDescriptor> Params;
   /// Flag to indicate if the function is valid.
   bool IsValid = false;
+  /// Flag to indicate if the function is done being
+  /// compiled to bytecode.
+  bool IsFullyCompiled = false;
 
 public:
   /// Dumps the disassembled bytecode to \c llvm::errs().
   void dump() const;
   void dump(llvm::raw_ostream &OS) const;
 };
 
 } // namespace interp
 } // namespace clang
 
 #endif

clang/test/AST/Interp/cxx20.cpp

 // RUN: %clang_cc1 -fexperimental-new-constant-interpreter -std=c++20 -verify %s
 // RUN: %clang_cc1 -std=c++20 -verify=ref %s
 
 
-// expected-no-diagnostics
-// ref-no-diagnostics
 constexpr int getMinus5() {
   int a = 10;
   a = -5;
   int *p = &a;
   return *p;
 }
 static_assert(getMinus5() == -5, "");
 
constexpr int pointerAssign2() {
 
   **pp = 12;
 
   int v = **pp;
 
   return v;
 }
 static_assert(pointerAssign2() == 12, "");
+
+
+constexpr int unInitLocal() {
+  int a;
+  return a; // ref-note{{read of uninitialized object}}
+}
+static_assert(unInitLocal() == 0, ""); // expected-error {{not an integral constant expression}} \
+                                       // ref-error {{not an integral constant expression}} \
+                                       // ref-note {{in call to 'unInitLocal()'}}