Skip to content

Expression#GetReference(): ref, not copy, index if possible #9579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Al2Klimov wants to merge 2 commits into
base: master
Choose a base branch
from
+77 −25
Open

Expression#GetReference(): ref, not copy, index if possible #9579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
73e2a36
Expression#GetReference(): ref, not copy, index if possible
Al2Klimov Nov 10, 2022
d072c6b
IndexerExpression#GetReference(): ref, not copy, index if it's a lite…
Al2Klimov Nov 10, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 36 additions & 21 deletions lib/config/expression.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,15 @@ using namespace icinga;
boost::signals2::signal<void (ScriptFrame&, ScriptError *ex, const DebugInfo&)> Expression::OnBreakpoint;
boost::thread_specific_ptr<bool> l_InBreakpointHandler;

RefIndex::RefIndex() : m_Foreign(&m_Own)
{ }

void RefIndex::Set(String own)
{
m_Own = std::move(own);
m_Foreign = &m_Own;
}

Expression::~Expression()
{ }

Expand Down Expand Up @@ -63,7 +72,7 @@ ExpressionResult Expression::Evaluate(ScriptFrame& frame, DebugHint *dhint) cons
}
}

bool Expression::GetReference(ScriptFrame&, [[maybe_unused]] bool init_dict, [[maybe_unused]] Value* parent, [[maybe_unused]] String* index, DebugHint**) const
bool Expression::GetReference(ScriptFrame&, [[maybe_unused]] bool init_dict, [[maybe_unused]] Value *parent, RefIndex*, DebugHint**) const
{
return false;
}
Expand Down Expand Up @@ -124,9 +133,9 @@ ExpressionResult VariableExpression::DoEvaluate(ScriptFrame& frame, DebugHint*)
BOOST_THROW_EXCEPTION(ScriptError{"Tried to access undefined script variable '" + m_Variable + "'"});
}

bool VariableExpression::GetReference(ScriptFrame& frame, [[maybe_unused]] bool init_dict, Value *parent, String *index, DebugHint **dhint) const
bool VariableExpression::GetReference(ScriptFrame& frame, [[maybe_unused]] bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const
{
*index = m_Variable;
index->Set(&m_Variable);

if (frame.Locals && frame.Locals->Contains(m_Variable)) {
*parent = frame.Locals;
Expand Down Expand Up @@ -154,15 +163,15 @@ bool VariableExpression::GetReference(ScriptFrame& frame, [[maybe_unused]] bool
ExpressionResult RefExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const
{
Value parent;
String index;
RefIndex index;

if (!m_Operand->GetReference(frame, false, &parent, &index, &dhint))
BOOST_THROW_EXCEPTION(ScriptError("Cannot obtain reference for expression.", m_DebugInfo));

if (!parent.IsObject())
BOOST_THROW_EXCEPTION(ScriptError("Cannot obtain reference for expression because parent is not an object.", m_DebugInfo));

return new Reference(parent, index);
return new Reference(parent, index.Get());
}

ExpressionResult DerefExpression::DoEvaluate(ScriptFrame& frame, DebugHint*) const
Expand All @@ -179,7 +188,7 @@ ExpressionResult DerefExpression::DoEvaluate(ScriptFrame& frame, DebugHint*) con
return ref->Get();
}

bool DerefExpression::GetReference(ScriptFrame& frame, [[maybe_unused]] bool init_dict, Value *parent, String *index, DebugHint**) const
bool DerefExpression::GetReference(ScriptFrame& frame, [[maybe_unused]] bool init_dict, Value *parent, RefIndex *index, DebugHint**) const
{
ExpressionResult operand = m_Operand->Evaluate(frame);
if (operand.GetCode() != ResultOK)
Expand All @@ -192,7 +201,7 @@ bool DerefExpression::GetReference(ScriptFrame& frame, [[maybe_unused]] bool ini
}

*parent = ref->GetParent();
*index = ref->GetIndex();
index->Set(ref->GetIndex());
return true;
}

Expand Down Expand Up @@ -455,10 +464,10 @@ ExpressionResult LogicalOrExpression::DoEvaluate(ScriptFrame& frame, DebugHint*)
ExpressionResult FunctionCallExpression::DoEvaluate(ScriptFrame& frame, DebugHint*) const
{
Value self, vfunc;
String index;
RefIndex index;

if (m_FName->GetReference(frame, false, &self, &index))
vfunc = VMOps::GetField(self, index, frame.Sandboxed, m_DebugInfo);
vfunc = VMOps::GetField(self, index.Get(), frame.Sandboxed, m_DebugInfo);
else {
ExpressionResult vfuncres = m_FName->Evaluate(frame);
CHECK_RESULT(vfuncres);
Expand Down Expand Up @@ -617,7 +626,7 @@ ExpressionResult SetExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint)
DebugHint *psdhint = dhint;

Value parent;
String index;
RefIndex index;

if (!m_Operand1->GetReference(frame, true, &parent, &index, &psdhint))
BOOST_THROW_EXCEPTION(ScriptError("Expression cannot be assigned to.", m_DebugInfo));
Expand All @@ -626,7 +635,7 @@ ExpressionResult SetExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint)
CHECK_RESULT(operand2);

if (m_Op != OpSetLiteral) {
Value object = VMOps::GetField(parent, index, frame.Sandboxed, m_DebugInfo);
Value object = VMOps::GetField(parent, index.Get(), frame.Sandboxed, m_DebugInfo);

switch (m_Op) {
case OpSetAdd:
Expand Down Expand Up @@ -658,7 +667,7 @@ ExpressionResult SetExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dhint)
}
}

VMOps::SetField(parent, index, operand2.GetValue(), m_DebugInfo);
VMOps::SetField(parent, index.Get(), operand2.GetValue(), m_DebugInfo);

if (psdhint) {
psdhint->AddMessage("=", m_DebugInfo);
Expand Down Expand Up @@ -746,10 +755,10 @@ ExpressionResult IndexerExpression::DoEvaluate(ScriptFrame& frame, DebugHint *dh
return VMOps::GetField(operand1.GetValue(), operand2.GetValue(), frame.Sandboxed, m_DebugInfo);
}

bool IndexerExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const
bool IndexerExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const
{
Value vparent;
String vindex;
RefIndex vindex;
DebugHint *psdhint = nullptr;
bool free_psd = false;

Expand All @@ -766,29 +775,35 @@ bool IndexerExpression::GetReference(ScriptFrame& frame, bool init_dict, Value *

if (vparent.IsObject()) {
Object::Ptr oparent = vparent;
has_field = oparent->HasOwnField(vindex);
has_field = oparent->HasOwnField(vindex.Get());
}

if (has_field)
old_value = VMOps::GetField(vparent, vindex, frame.Sandboxed, m_Operand1->GetDebugInfo());
old_value = VMOps::GetField(vparent, vindex.Get(), frame.Sandboxed, m_Operand1->GetDebugInfo());

if (old_value.IsEmpty() && !old_value.IsString())
VMOps::SetField(vparent, vindex, new Dictionary(), m_Operand1->GetDebugInfo());
VMOps::SetField(vparent, vindex.Get(), new Dictionary(), m_Operand1->GetDebugInfo());
}

*parent = VMOps::GetField(vparent, vindex, frame.Sandboxed, m_DebugInfo);
*parent = VMOps::GetField(vparent, vindex.Get(), frame.Sandboxed, m_DebugInfo);
free_psd = true;
} else {
ExpressionResult operand1 = m_Operand1->Evaluate(frame);
*parent = operand1.GetValue();
}

ExpressionResult operand2 = m_Operand2->Evaluate(frame);
*index = operand2.GetValue();
auto lit (dynamic_cast<LiteralExpression*>(m_Operand2.get()));

if (lit && lit->GetValue().IsString()) {
index->Set(&lit->GetValue().Get<String>());
} else {
ExpressionResult operand2 = m_Operand2->Evaluate(frame);
index->Set(operand2.GetValue());
}

if (dhint) {
if (psdhint)
*dhint = new DebugHint(psdhint->GetChild(*index));
*dhint = new DebugHint(psdhint->GetChild(index->Get()));
else
*dhint = nullptr;
}
Expand Down
45 changes: 41 additions & 4 deletions lib/config/expression.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,37 @@
if (res.GetCode() == ResultBreak) \
break; \

/**
* Abstracts a maybe owned String
*
* @ingroup config
*/
class RefIndex
{
public:
RefIndex();
RefIndex(const RefIndex&) = delete;
RefIndex(RefIndex&&) = delete;
RefIndex& operator=(const RefIndex&) = delete;
RefIndex& operator=(RefIndex&&) = delete;

inline const String& Get() const noexcept
{
return *m_Foreign;
}

inline void Set(const String* foreign) noexcept
{
m_Foreign = foreign;
}

void Set(String own);

private:
const String* m_Foreign;
String m_Own;
};

/**
* @ingroup config
*/
Expand All @@ -191,7 +222,13 @@
Expression& operator=(const Expression&) = delete;

ExpressionResult Evaluate(ScriptFrame& frame, DebugHint *dhint = nullptr) const;
virtual bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint = nullptr) const;

/**
* Caution! On return `*index` may point to a String somewhere in `this` (or subAST).
* Keep `this` alive while using `*index`!
*/
virtual bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint = nullptr) const;

virtual const DebugInfo& GetDebugInfo() const;

virtual ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const = 0;
Expand Down Expand Up @@ -260,7 +297,7 @@
{ }

protected:
const DebugInfo& GetDebugInfo() const final;

Check warning on line 300 in lib/config/expression.hpp

View workflow job for this annotation

GitHub Actions / opensuse/leap:15.6 

'virtual const icinga::DebugInfo& icinga::DebuggableExpression::GetDebugInfo() const' can be marked override [-Wsuggest-override]

Check warning on line 300 in lib/config/expression.hpp

View workflow job for this annotation

GitHub Actions / rockylinux:8 

'virtual const icinga::DebugInfo& icinga::DebuggableExpression::GetDebugInfo() const' can be marked override [-Wsuggest-override]

Check warning on line 300 in lib/config/expression.hpp

View workflow job for this annotation

GitHub Actions / rockylinux:8 

'virtual const icinga::DebugInfo& icinga::DebuggableExpression::GetDebugInfo() const' can be marked override [-Wsuggest-override]

Check warning on line 300 in lib/config/expression.hpp

View workflow job for this annotation

GitHub Actions / rockylinux:8 

'virtual const icinga::DebugInfo& icinga::DebuggableExpression::GetDebugInfo() const' can be marked override [-Wsuggest-override]

Check warning on line 300 in lib/config/expression.hpp

View workflow job for this annotation

GitHub Actions / rockylinux:8 

'virtual const icinga::DebugInfo& icinga::DebuggableExpression::GetDebugInfo() const' can be marked override [-Wsuggest-override]

Check warning on line 300 in lib/config/expression.hpp

View workflow job for this annotation

GitHub Actions / registry.suse.com/suse/sle15:15.6 

'virtual const icinga::DebugInfo& icinga::DebuggableExpression::GetDebugInfo() const' can be marked override [-Wsuggest-override]

Check warning on line 300 in lib/config/expression.hpp

View workflow job for this annotation

GitHub Actions / registry.suse.com/suse/sle15:15.7 

'virtual const icinga::DebugInfo& icinga::DebuggableExpression::GetDebugInfo() const' can be marked override [-Wsuggest-override]

DebugInfo m_DebugInfo;
};
Expand Down Expand Up @@ -310,7 +347,7 @@

protected:
ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const override;
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const override;

private:
String m_Variable;
Expand All @@ -328,7 +365,7 @@

protected:
ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const override;
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const override;
};

class RefExpression final : public UnaryExpression
Expand Down Expand Up @@ -754,7 +791,7 @@

protected:
ExpressionResult DoEvaluate(ScriptFrame& frame, DebugHint *dhint) const override;
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, String *index, DebugHint **dhint) const override;
bool GetReference(ScriptFrame& frame, bool init_dict, Value *parent, RefIndex *index, DebugHint **dhint) const override;

friend void BindToScope(std::unique_ptr<Expression>& expr, ScopeSpecifier scopeSpec);
};
Expand Down
Loading