Skip to content

Commit

Permalink
libnixf: return non-owned references for Attribute (#371)
Browse files Browse the repository at this point in the history
  • Loading branch information
inclyc authored Mar 29, 2024
1 parent f1d754a commit 93c0f22
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 37 deletions.
9 changes: 2 additions & 7 deletions libnixf/include/nixf/Basic/Nodes/Attrs.h
Original file line number Diff line number Diff line change
Expand Up @@ -198,20 +198,15 @@ class Attribute {
bool FromInherit;

public:
Attribute() = default;
Attribute(std::shared_ptr<Node> Key, std::shared_ptr<Expr> Value,
bool FromInherit)
: Key(std::move(Key)), Value(std::move(Value)), FromInherit(FromInherit) {
assert(this->Key && "Key must not be null");
}

[[nodiscard]] std::shared_ptr<Node> &key() { return Key; }
[[nodiscard]] Node &key() const { return *Key; }

[[nodiscard]] const std::shared_ptr<Node> &key() const { return Key; }

[[nodiscard]] std::shared_ptr<Expr> &value() { return Value; }

[[nodiscard]] const std::shared_ptr<Expr> &value() const { return Value; }
[[nodiscard]] Expr *value() const { return Value.get(); }

[[nodiscard]] bool fromInherit() const { return FromInherit; }
};
Expand Down
23 changes: 12 additions & 11 deletions libnixf/src/Sema/SemaActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ void Sema::mergeAttrSets(SemaAttrs &XAttrs, const SemaAttrs &YAttrs) {
}
*/
dupAttr(K, V.key()->range(), XAttrs.Static.at(K).key()->range());
dupAttr(K, V.key().range(), XAttrs.Static.at(K).key().range());
continue;
}
XAttrs.Static.insert({K, V});
Expand Down Expand Up @@ -85,18 +85,19 @@ void Sema::insertAttr(SemaAttrs &SA, std::shared_ptr<AttrName> Name,
if (V.value() && V.value()->kind() == Node::NK_ExprAttrs && E &&
E->kind() == Node::NK_ExprAttrs) {
// If this is also an attrset, we want to merge them.
auto *XAttrSet = static_cast<ExprAttrs *>(V.value().get());
auto *XAttrSet = static_cast<ExprAttrs *>(V.value());
auto *YAttrSet = static_cast<ExprAttrs *>(E.get());
checkAttrRecursiveForMerge(*XAttrSet, *YAttrSet);
mergeAttrSets(XAttrSet->SA, YAttrSet->SA);
return;
}
dupAttr(StaticName, Name->range(), V.key()->range());
dupAttr(StaticName, Name->range(), V.key().range());
return;
}
if (!E)
return;
Attrs[StaticName] = Attribute(std::move(Name), std::move(E), IsInherit);
Attrs.insert(
{StaticName, Attribute(std::move(Name), std::move(E), IsInherit)});
}

SemaAttrs *
Expand All @@ -122,19 +123,19 @@ Sema::selectOrCreate(SemaAttrs &SA,
const auto &[K, V] = *Nested;
if (V.fromInherit() || !V.value() ||
V.value()->kind() != Node::NK_ExprAttrs) {
dupAttr(StaticName, Name->range(), V.key()->range());
dupAttr(StaticName, Name->range(), V.key().range());
return nullptr;
}
Inner = &static_cast<ExprAttrs *>(V.value().get())->SA;
Inner = &static_cast<ExprAttrs *>(V.value())->SA;
} else {
// There is no existing one, let's create a new attribute.
// These attributes are implicitly created, and to match default ctor
// in C++ nix implementation, they are all non-recursive.
auto NewNested = std::make_shared<ExprAttrs>(
Name->range(), nullptr, nullptr, SemaAttrs(/*Recursive=*/nullptr));
Inner = &NewNested->SA;
StaticAttrs[StaticName] =
Attribute(Name, std::move(NewNested), /*FromInherit=*/false);
StaticAttrs.insert({StaticName, Attribute(Name, std::move(NewNested),
/*FromInherit=*/false)});
}
} else {
// Create a dynamic attribute.
Expand Down Expand Up @@ -284,13 +285,13 @@ void Sema::lowerInheritName(SemaAttrs &SA, std::shared_ptr<AttrName> Name,
// Check duplicated attrname.
if (SA.Static.contains(Name->staticName())) {
dupAttr(Name->staticName(), Name->range(),
SA.Static[Name->staticName()].key()->range());
SA.Static.at(Name->staticName()).key().range());
return;
}
// Insert the attr.
std::string StaticName = Name->staticName();
SA.Static[StaticName] =
Attribute(std::move(Name), std::move(E), /*FromInherit=*/true);
SA.Static.insert({StaticName, Attribute(std::move(Name), std::move(E),
/*FromInherit=*/true)});
}

void Sema::lowerInherit(SemaAttrs &Attr, const Inherit &Inherit) {
Expand Down
45 changes: 26 additions & 19 deletions libnixf/test/Sema/SemaActions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ TEST_F(SemaActionTest, selectOrCreate) {

ASSERT_EQ(Inner.value()->kind(), Node::NK_ExprAttrs);

const auto *InnerAttr =
&static_cast<ExprAttrs *>(Inner.value().get())->sema();
const auto *InnerAttr = &static_cast<ExprAttrs *>(Inner.value())->sema();
ASSERT_EQ(InnerAttr->staticAttrs().size(), 1);
ASSERT_EQ(InnerAttr->staticAttrs().count("b"), 1);
ASSERT_EQ(InnerAttr->staticAttrs().at("b").value()->kind(),
Expand All @@ -59,7 +58,7 @@ TEST_F(SemaActionTest, selectOrCreate) {
ASSERT_EQ(InnerAttr->staticAttrs().at("b").value()->kind(),
Node::NK_ExprAttrs);
InnerAttr =
&static_cast<ExprAttrs *>(InnerAttr->staticAttrs().at("b").value().get())
&static_cast<ExprAttrs *>(InnerAttr->staticAttrs().at("b").value())
->sema();
ASSERT_EQ(InnerAttr->staticAttrs().size(), 0);

Expand All @@ -72,14 +71,14 @@ TEST_F(SemaActionTest, selectOrCreate) {

ASSERT_EQ(Inner.value()->kind(), Node::NK_ExprAttrs);

InnerAttr = &static_cast<ExprAttrs *>(Inner.value().get())->sema();
InnerAttr = &static_cast<ExprAttrs *>(Inner.value())->sema();
ASSERT_EQ(InnerAttr->staticAttrs().size(), 1);
ASSERT_EQ(InnerAttr->staticAttrs().count("b"), 1);
ASSERT_EQ(InnerAttr->staticAttrs().at("b").value()->kind(),
Node::NK_ExprAttrs);

InnerAttr =
&static_cast<ExprAttrs *>(InnerAttr->staticAttrs().at("b").value().get())
&static_cast<ExprAttrs *>(InnerAttr->staticAttrs().at("b").value())
->sema();
ASSERT_EQ(InnerAttr->staticAttrs().size(), 0);
}
Expand All @@ -99,17 +98,19 @@ TEST_F(SemaActionTest, selectOrCreateDynamic) {

const auto &DynamicAttr = Attr.dynamicAttrs().front();

ASSERT_EQ(DynamicAttr.key()->kind(), Node::NK_AttrName);
ASSERT_EQ(DynamicAttr.key().kind(), Node::NK_AttrName);
}

TEST_F(SemaActionTest, insertAttrDup) {
auto Name = getStaticName("a");
// Check we can detect duplicated attr.
std::map<std::string, Attribute> Attrs;

Attrs["a"] = Attribute(
/*Key=*/Name, /*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false);
Attrs.insert(
{"a", Attribute(
/*Key=*/Name,
/*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false)});

SemaAttrs A(std::move(Attrs), {}, nullptr);
std::shared_ptr<ExprInt> E(new ExprInt{{}, 1});
Expand Down Expand Up @@ -183,9 +184,11 @@ TEST_F(SemaActionTest, inheritNameDuplicated) {
auto Name = getStaticName("a");
std::map<std::string, Attribute> Attrs;

Attrs["a"] = Attribute(
/*Key=*/Name, /*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false);
Attrs.insert(
{"a", Attribute(
/*Key=*/Name,
/*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false)});

SemaAttrs SA(std::move(Attrs), {}, nullptr);
L.lowerInheritName(SA, Name, nullptr);
Expand All @@ -205,13 +208,17 @@ TEST_F(SemaActionTest, mergeAttrSets) {
std::map<std::string, Attribute> XAttrs;
std::map<std::string, Attribute> YAttrs;

XAttrs["a"] = Attribute(
/*Key=*/XName, /*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false);

YAttrs["a"] = Attribute(
/*Key=*/YName, /*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false);
XAttrs.insert(
{"a", Attribute(
/*Key=*/XName,
/*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false)});

YAttrs.insert(
{"a", Attribute(
/*Key=*/YName,
/*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false)});

SemaAttrs XA(XAttrs, {}, nullptr);
SemaAttrs YA(YAttrs, {}, nullptr);
Expand Down

0 comments on commit 93c0f22

Please sign in to comment.