cleaner approach for localizing errors associated with duplicated ASTs: virtualize GetLocationInfo

src/Expr.cc

@@ -277,7 +277,7 @@ void Expr::ExprError(const char msg[])
 
 void Expr::RuntimeError(const std::string& msg) const
 	{
-	reporter->ExprRuntimeError(Original(), "%s", msg.data());
+	reporter->ExprRuntimeError(this, "%s", msg.data());
 	}
 
 void Expr::RuntimeErrorWithCallStack(const std::string& msg) const
@@ -285,13 +285,13 @@ void Expr::RuntimeErrorWithCallStack(const std::string& msg) const
 	auto rcs = render_call_stack();
 
 	if ( rcs.empty() )
-		reporter->ExprRuntimeError(Original(), "%s", msg.data());
+		reporter->ExprRuntimeError(this, "%s", msg.data());
 	else
 		{
 		ODesc d;
 		d.SetShort();
 		Describe(&d);
-		reporter->RuntimeError(Original()->GetLocationInfo(),
+		reporter->RuntimeError(GetLocationInfo(),
 					"%s, expression: %s, call stack: %s",
 					msg.data(), d.Description(), rcs.data());
 		}

src/Expr.h

@@ -228,6 +228,14 @@ public:
 		return {AdoptRef{}, succ};
 		}
 
+	const detail::Location* GetLocationInfo() const override
+		{
+		if ( original )
+			return original->GetLocationInfo();
+		else
+			return Obj::GetLocationInfo();
+		}
+
 protected:
 	Expr() = default;
 	explicit Expr(BroExprTag arg_tag);

src/Func.cc

@@ -115,7 +115,7 @@ std::string render_call_stack()
 
 		if ( ci.call )
 			{
-			auto loc = ci.call->Original()->GetLocationInfo();
+			auto loc = ci.call->GetLocationInfo();
 			rval += util::fmt(" at %s:%d", loc->filename, loc->first_line);
 			}
 
@@ -385,8 +385,7 @@ ValPtr ScriptFunc::Invoke(zeek::Args* args, Frame* parent) const
 	for ( const auto& body : bodies )
 		{
 		if ( sample_logger )
-			sample_logger->LocationSeen(
+			sample_logger->LocationSeen(body.stmts->GetLocationInfo());
-				body.stmts->Original()->GetLocationInfo());
 
 		// Fill in the rest of the frame with the function's arguments.
 		for ( auto j = 0u; j < args->size(); ++j )

src/Obj.h

@@ -110,7 +110,7 @@ public:
 	void AddLocation(ODesc* d) const;
 
 	// Get location info for debugging.
-	const detail::Location* GetLocationInfo() const
+	virtual const detail::Location* GetLocationInfo() const
 		{ return location ? location : &detail::no_location; }
 
 	virtual bool SetLocationInfo(const detail::Location* loc)

src/ScriptCoverageManager.cc

@@ -133,7 +133,7 @@ bool ScriptCoverageManager::WriteStats()
 	      it != stmts.end(); ++it )
 		{
 		ODesc location_info;
-		(*it)->Original()->GetLocationInfo()->Describe(&location_info);
+		(*it)->GetLocationInfo()->Describe(&location_info);
 		ODesc desc_info;
 		(*it)->Describe(&desc_info);
 		string desc(desc_info.Description());

src/Stmt.cc

@@ -107,8 +107,7 @@ bool Stmt::SetLocationInfo(const Location* start, const Location* end)
 
 	Filemap& map = *(map_iter->second);
 
-	StmtLocMapping* new_mapping =
+	StmtLocMapping* new_mapping = new StmtLocMapping(GetLocationInfo(), this);
-		new StmtLocMapping(Original()->GetLocationInfo(), this);
 
 	// Optimistically just put it at the end.
 	map.push_back(new_mapping);
@@ -193,7 +192,7 @@ ExprListStmt::ExprListStmt(StmtTag t, ListExprPtr arg_l)
 			Error("value of type void illegal");
 		}
 
-	SetLocationInfo(l->Original()->GetLocationInfo());
+	SetLocationInfo(l->GetLocationInfo());
 	}
 
 ExprListStmt::~ExprListStmt() = default;
@@ -344,13 +343,13 @@ ExprStmt::ExprStmt(ExprPtr arg_e) : Stmt(STMT_EXPR), e(std::move(arg_e))
 	if ( e && e->IsPure() )
 		Warn("expression value ignored");
 
-	SetLocationInfo(e->Original()->GetLocationInfo());
+	SetLocationInfo(e->GetLocationInfo());
 	}
 
 ExprStmt::ExprStmt(StmtTag t, ExprPtr arg_e) : Stmt(t), e(std::move(arg_e))
 	{
 	if ( e )
-		SetLocationInfo(e->Original()->GetLocationInfo());
+		SetLocationInfo(e->GetLocationInfo());
 	}
 
 ExprStmt::~ExprStmt() = default;
@@ -423,8 +422,8 @@ IfStmt::IfStmt(ExprPtr test,
 	if ( ! e->IsError() && ! IsBool(e->GetType()->Tag()) )
 		e->Error("conditional in test must be boolean");
 
-	const Location* loc1 = s1->Original()->GetLocationInfo();
+	const Location* loc1 = s1->GetLocationInfo();
-	const Location* loc2 = s2->Original()->GetLocationInfo();
+	const Location* loc2 = s2->GetLocationInfo();
 	SetLocationInfo(loc1, loc2);
 	}
 
@@ -805,7 +804,7 @@ bool SwitchStmt::AddCaseLabelValueMapping(const Val* v, int idx)
 
 	if ( ! hk )
 		{
-		reporter->PushLocation(e->Original()->GetLocationInfo());
+		reporter->PushLocation(e->GetLocationInfo());
 		reporter->InternalError("switch expression type mismatch (%s/%s)",
 		                        type_name(v->GetType()->Tag()),
 		                        type_name(e->GetType()->Tag()));
@@ -846,7 +845,7 @@ std::pair<int, ID*> SwitchStmt::FindCaseLabelMatch(const Val* v) const
 
 		if ( ! hk )
 			{
-			reporter->PushLocation(e->Original()->GetLocationInfo());
+			reporter->PushLocation(e->GetLocationInfo());
 			reporter->Error("switch expression type mismatch (%s/%s)",
 			                type_name(v->GetType()->Tag()),
 			                type_name(e->GetType()->Tag()));

src/StmtBase.h

@@ -113,6 +113,14 @@ public:
 		return {AdoptRef{}, succ};
 		}
 
+	const detail::Location* GetLocationInfo() const override
+		{
+		if ( original )
+			return original->GetLocationInfo();
+		else
+			return Obj::GetLocationInfo();
+		}
+
 protected:
 	explicit Stmt(StmtTag arg_tag);
 

src/reporter.bif

@@ -22,7 +22,7 @@ module Reporter;
 ## .. zeek:see:: reporter_info
 function Reporter::info%(msg: string%): bool
 	%{
-	reporter->PushLocation(frame->GetCall()->Original()->GetLocationInfo());
+	reporter->PushLocation(frame->GetCall()->GetLocationInfo());
 	reporter->Info("%s", msg->CheckString());
 	reporter->PopLocation();
 	return zeek::val_mgr->True();
@@ -37,7 +37,7 @@ function Reporter::info%(msg: string%): bool
 ## .. zeek:see:: reporter_warning
 function Reporter::warning%(msg: string%): bool
 	%{
-	reporter->PushLocation(frame->GetCall()->Original()->GetLocationInfo());
+	reporter->PushLocation(frame->GetCall()->GetLocationInfo());
 	reporter->Warning("%s", msg->CheckString());
 	reporter->PopLocation();
 	return zeek::val_mgr->True();
@@ -53,7 +53,7 @@ function Reporter::warning%(msg: string%): bool
 ## .. zeek:see:: reporter_error
 function Reporter::error%(msg: string%): bool
 	%{
-	reporter->PushLocation(frame->GetCall()->Original()->GetLocationInfo());
+	reporter->PushLocation(frame->GetCall()->GetLocationInfo());
 	reporter->Error("%s", msg->CheckString());
 	reporter->PopLocation();
 	return zeek::val_mgr->True();
@@ -66,7 +66,7 @@ function Reporter::error%(msg: string%): bool
 ## Returns: Always true.
 function Reporter::fatal%(msg: string%): bool
 	%{
-	reporter->PushLocation(frame->GetCall()->Original()->GetLocationInfo());
+	reporter->PushLocation(frame->GetCall()->GetLocationInfo());
 	reporter->FatalError("%s", msg->CheckString());
 	reporter->PopLocation();
 	return zeek::val_mgr->True();
@@ -80,7 +80,7 @@ function Reporter::fatal%(msg: string%): bool
 ## Returns: Always true.
 function Reporter::fatal_error_with_core%(msg: string%): bool
 	%{
-	reporter->PushLocation(frame->GetCall()->Original()->GetLocationInfo());
+	reporter->PushLocation(frame->GetCall()->GetLocationInfo());
 	reporter->FatalErrorWithCore("%s", msg->CheckString());
 	reporter->PopLocation();
 	return zeek::val_mgr->True();

src/zeek.bif

@@ -2333,7 +2333,7 @@ function backtrace%(%): Backtrace
 
 		if ( ci.call )
 			{
-			auto loc = ci.call->Original()->GetLocationInfo();
+			auto loc = ci.call->GetLocationInfo();
 			elem->Assign<StringVal>(file_location_idx, loc->filename);
 			elem->Assign(line_location_idx, val_mgr->Count(loc->first_line));
 			}

testing/btest/Baseline.dup/broker.store.invalid-handle/out

@@ -1,4 +0,0 @@
-### BTest baseline data generated by btest-diff. Do not edit. Use "btest -U/-u" to update. Requires BTest >= 0.63.
-expression error in <...>/invalid-handle.zeek, lines 16-22: invalid Broker store handle (0), during call: (Broker::is_closed(a))
-error in <...>/invalid-handle.zeek, lines 4-14: invalid Broker store handle (Broker::keys(a) and 0)
-keys, [status=Broker::FAILURE, result=[data=<uninitialized>]]

testing/btest/btest.cfg

@@ -48,12 +48,6 @@ UBSAN_OPTIONS=print_stacktrace=1
 #	When lambdas are duplicated they get a new UID, which differs
 #	from the original.
 #
-# broker.store.invalid-handle
-#	Line numbers in some error messages differ.  The duplicated
-#	ones are "wider" (entire function) than the originals.  Most
-#	such differences have been already fixed; this one is a bit
-#	puzzling, but doesn't seem worth trying to fix.
-#
 # coverage.zeek-profiler-file
 #	Not sure what's going on here, but best guess the problem is
 #	that the coverage tracking is looking for execution of the