[clang][bytecode] Disable location tracking for implicit field inits

[tbaederr]

No description provided.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Timm Baeder (tbaederr)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/150190.diff

12 Files Affected:

• (modified) clang/lib/AST/ByteCode/ByteCodeEmitter.cpp (+3-1)
• (modified) clang/lib/AST/ByteCode/ByteCodeEmitter.h (+1)
• (modified) clang/lib/AST/ByteCode/Compiler.cpp (+31)
• (modified) clang/lib/AST/ByteCode/Compiler.h (+2)
• (modified) clang/lib/AST/ByteCode/EvalEmitter.cpp (+1)
• (modified) clang/lib/AST/ByteCode/EvalEmitter.h (+1)
• (modified) clang/lib/AST/ByteCode/InterpFrame.cpp (+12-1)
• (modified) clang/lib/AST/ByteCode/State.cpp (+1)
• (modified) clang/test/AST/ByteCode/lifetimes.cpp (+1-2)
• (modified) clang/test/AST/ByteCode/mutable.cpp (+8)
• (modified) clang/test/AST/ByteCode/new-delete.cpp (+2-3)
• (modified) clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp (+3)

diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
index 3288585683c10..b20b025f17007 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.cpp
@@ -217,7 +217,9 @@ bool ByteCodeEmitter::emitOp(Opcode Op, const Tys &...Args,
   // The opcode is followed by arguments. The source info is
   // attached to the address after the opcode.
   emit(P, Code, Op, Success);
-  if (SI)
+  if (LocOverride)
+    SrcMap.emplace_back(Code.size(), *LocOverride);
+  else if (SI)
     SrcMap.emplace_back(Code.size(), SI);
 
   (..., emit(P, Code, Args, Success));
diff --git a/clang/lib/AST/ByteCode/ByteCodeEmitter.h b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
index 9e9dd5e87cd7a..fafba3f68765c 100644
--- a/clang/lib/AST/ByteCode/ByteCodeEmitter.h
+++ b/clang/lib/AST/ByteCode/ByteCodeEmitter.h
@@ -73,6 +73,7 @@ class ByteCodeEmitter {
   ParamOffset LambdaThisCapture{0, false};
   /// Local descriptors.
   llvm::SmallVector<SmallVector<Local, 8>, 2> Descriptors;
+  std::optional<SourceInfo> LocOverride = std::nullopt;
 
 private:
   /// Current compilation context.
diff --git a/clang/lib/AST/ByteCode/Compiler.cpp b/clang/lib/AST/ByteCode/Compiler.cpp
index e760055a8d235..b2dc17d866545 100644
--- a/clang/lib/AST/ByteCode/Compiler.cpp
+++ b/clang/lib/AST/ByteCode/Compiler.cpp
@@ -229,6 +229,31 @@ template <class Emitter> class StmtExprScope final {
   bool OldFlag;
 };
 
+/// When generating code for e.g. implicit field initializers in constructors,
+/// we don't have anything to point to in case the initializer causes an error.
+/// In that case, we need to disable location tracking for the initializer so
+/// we later point to the call range instead.
+template <class Emitter> class LocOverrideScope final {
+public:
+  LocOverrideScope(Compiler<Emitter> *Ctx, SourceInfo NewValue,
+                   bool Enabled = true)
+      : Ctx(Ctx), OldFlag(Ctx->LocOverride), Enabled(Enabled) {
+
+    if (Enabled)
+      Ctx->LocOverride = NewValue;
+  }
+
+  ~LocOverrideScope() {
+    if (Enabled)
+      Ctx->LocOverride = OldFlag;
+  }
+
+private:
+  Compiler<Emitter> *Ctx;
+  std::optional<SourceInfo> OldFlag;
+  bool Enabled;
+};
+
 } // namespace interp
 } // namespace clang
 
@@ -6000,6 +6025,10 @@ bool Compiler<Emitter>::compileConstructor(const CXXConstructorDecl *Ctor) {
     if (const FieldDecl *Member = Init->getMember()) {
       const Record::Field *F = R->getField(Member);
 
+      LocOverrideScope<Emitter> LOS(this, SourceInfo{},
+                                    !Init->isWritten() &&
+                                        !Init->isInClassMemberInitializer() &&
+                                        !isa<CXXConstructExpr>(InitExpr));
       if (!emitFieldInitializer(F, F->Offset, InitExpr, IsUnion))
         return false;
     } else if (const Type *Base = Init->getBaseClass()) {
@@ -6106,6 +6135,8 @@ bool Compiler<Emitter>::compileDestructor(const CXXDestructorDecl *Dtor) {
 
   assert(R);
   if (!R->isUnion()) {
+
+    LocOverrideScope<Emitter> LOS(this, SourceInfo{});
     // First, destroy all fields.
     for (const Record::Field &Field : llvm::reverse(R->fields())) {
       const Descriptor *D = Field.Desc;
diff --git a/clang/lib/AST/ByteCode/Compiler.h b/clang/lib/AST/ByteCode/Compiler.h
index 503269399c757..7933580f8128d 100644
--- a/clang/lib/AST/ByteCode/Compiler.h
+++ b/clang/lib/AST/ByteCode/Compiler.h
@@ -41,6 +41,7 @@ template <class Emitter> class LoopScope;
 template <class Emitter> class LabelScope;
 template <class Emitter> class SwitchScope;
 template <class Emitter> class StmtExprScope;
+template <class Emitter> class LocOverrideScope;
 
 template <class Emitter> class Compiler;
 struct InitLink {
@@ -334,6 +335,7 @@ class Compiler : public ConstStmtVisitor<Compiler<Emitter>, bool>,
   friend class LabelScope<Emitter>;
   friend class SwitchScope<Emitter>;
   friend class StmtExprScope<Emitter>;
+  friend class LocOverrideScope<Emitter>;
 
   /// Emits a zero initializer.
   bool visitZeroInitializer(PrimType T, QualType QT, const Expr *E);
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.cpp b/clang/lib/AST/ByteCode/EvalEmitter.cpp
index 81ebc5694d6f0..5ea0045cc24f2 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.cpp
+++ b/clang/lib/AST/ByteCode/EvalEmitter.cpp
@@ -53,6 +53,7 @@ EvaluationResult EvalEmitter::interpretDecl(const VarDecl *VD,
                                             bool CheckFullyInitialized) {
   this->CheckFullyInitialized = CheckFullyInitialized;
   S.EvaluatingDecl = VD;
+  S.setEvalLocation(VD->getLocation());
   EvalResult.setSource(VD);
 
   if (const Expr *Init = VD->getAnyInitializer()) {
diff --git a/clang/lib/AST/ByteCode/EvalEmitter.h b/clang/lib/AST/ByteCode/EvalEmitter.h
index 2fe7da608c739..85a0a99fbb4b0 100644
--- a/clang/lib/AST/ByteCode/EvalEmitter.h
+++ b/clang/lib/AST/ByteCode/EvalEmitter.h
@@ -95,6 +95,7 @@ class EvalEmitter : public SourceMapper {
   ParamOffset LambdaThisCapture{0, false};
   /// Local descriptors.
   llvm::SmallVector<SmallVector<Local, 8>, 2> Descriptors;
+  std::optional<SourceInfo> LocOverride = std::nullopt;
 
 private:
   /// Current compilation context.
diff --git a/clang/lib/AST/ByteCode/InterpFrame.cpp b/clang/lib/AST/ByteCode/InterpFrame.cpp
index d62a4f6275b50..fd333b3c21655 100644
--- a/clang/lib/AST/ByteCode/InterpFrame.cpp
+++ b/clang/lib/AST/ByteCode/InterpFrame.cpp
@@ -202,7 +202,18 @@ SourceRange InterpFrame::getCallRange() const {
       return NullRange;
     return S.EvalLocation;
   }
-  return S.getRange(Caller->Func, RetPC - sizeof(uintptr_t));
+
+  // Move up to the frame that has a valid location for the caller.
+  CodePtr Ret = RetPC;
+  for (const InterpFrame *C = this; C; C = C->Caller) {
+    if (!C->RetPC)
+      continue;
+    SourceRange CallRange =
+        S.getRange(C->Caller->Func, C->RetPC - sizeof(uintptr_t));
+    if (CallRange.isValid())
+      return CallRange;
+  }
+  return S.EvalLocation;
 }
 
 const FunctionDecl *InterpFrame::getCallee() const {
diff --git a/clang/lib/AST/ByteCode/State.cpp b/clang/lib/AST/ByteCode/State.cpp
index 3204d1a193a74..dc3d0da7a4a46 100644
--- a/clang/lib/AST/ByteCode/State.cpp
+++ b/clang/lib/AST/ByteCode/State.cpp
@@ -131,6 +131,7 @@ void State::addCallStack(unsigned Limit) {
   const Frame *Bottom = getBottomFrame();
   for (const Frame *F = Top; F != Bottom; F = F->getCaller(), ++CallIdx) {
     SourceRange CallRange = F->getCallRange();
+    assert(CallRange.isValid());
 
     // Skip this call?
     if (CallIdx >= SkipStart && CallIdx < SkipEnd) {
diff --git a/clang/test/AST/ByteCode/lifetimes.cpp b/clang/test/AST/ByteCode/lifetimes.cpp
index 5e021387348b5..5c8d56291f079 100644
--- a/clang/test/AST/ByteCode/lifetimes.cpp
+++ b/clang/test/AST/ByteCode/lifetimes.cpp
@@ -31,8 +31,7 @@ struct S {
                                 // expected-note {{read of temporary whose lifetime has ended}}
 };
 constexpr int k1 = S().t; // both-error {{must be initialized by a constant expression}} \
-                          // ref-note {{in call to}} \
-                          // expected-note {{in call to}}
+                          // both-note {{in call to}}
 
 
 namespace MoveFnWorks {
diff --git a/clang/test/AST/ByteCode/mutable.cpp b/clang/test/AST/ByteCode/mutable.cpp
index 35c5a0389921e..f9ea71f68bc40 100644
--- a/clang/test/AST/ByteCode/mutable.cpp
+++ b/clang/test/AST/ByteCode/mutable.cpp
@@ -66,3 +66,11 @@ namespace MutableInConst {
   static_assert(mutableInConst() == 1, "");
 }
 #endif
+
+struct D { mutable int y; }; // both-note {{declared here}}
+constexpr D d1 = { 1 };
+constexpr D d2 = d1; // both-error {{must be initialized by a constant expression}} \
+                     // both-note {{read of mutable member 'y}}
+                     // both-note {{in call to}}
+
+
diff --git a/clang/test/AST/ByteCode/new-delete.cpp b/clang/test/AST/ByteCode/new-delete.cpp
index 3c0bdbc8c99fe..c5f1878c41734 100644
--- a/clang/test/AST/ByteCode/new-delete.cpp
+++ b/clang/test/AST/ByteCode/new-delete.cpp
@@ -546,14 +546,13 @@ namespace FaultyDtorCalledByDelete {
       a = new int(13);
       IF.mem = new int(100);
     }
-    constexpr ~Foo() { delete a; } // expected-note {{in call to}}
+    constexpr ~Foo() { delete a; }
   };
 
   constexpr int abc() {
     Foo *F = new Foo();
     int n = *F->a;
-    delete F; // both-note {{in call to}} \
-              // ref-note {{in call to}}
+    delete F; // both-note 2{{in call to}}
 
     return n;
   }
diff --git a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
index 270bc3cb48be9..dd0f1fc3cba8e 100644
--- a/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp
@@ -1,6 +1,9 @@
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s
 // RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s
 
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++11 %s -fexperimental-new-constant-interpreter
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s -fexperimental-new-constant-interpreter
+
 // A constexpr specifier used in an object declaration declares the object as
 // const.
 constexpr int a = 0;