Skip to content

Commit

Permalink
libnixf/Sema: fix attribute source context of inherit (expr) ... (#514
Browse files Browse the repository at this point in the history 
)

Consider recursive attributes:

    rec {
        inherit (expr) attr;
        inherit attr;
    }

The syntax:

    inherit (expr) attr

is desugared into

    attr = expr.attr

.

The expression `expr.attr` should be evaluated in new environment,
created by `rec` keyword

However,

    inherit attr

desugaring to

    attr = attr

should be evaluated in parent environment.

This scoping rule is somehow straightfoward but not really documented.
libnixf previously evaluated the second case in "new environment".

Fixes: #513
  • Loading branch information
@inclyc
inclyc authored May 29, 2024
1 parent fe20230 commit 2d37fb2
Show file tree
Hide file tree
Showing 6 changed files with 86 additions and 39 deletions.
23 changes: 19 additions & 4 deletions libnixf/include/nixf/Basic/Nodes/Attrs.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -187,22 +187,37 @@ class Binds : public Node {
};

class Attribute {
public:
enum class AttributeKind {
/// a = b;
Plain,
/// inherit a b c;
Inherit,
/// inherit (expr) a b c
InheritFrom,
};

private:
const std::shared_ptr<Node> Key;
const std::shared_ptr<Expr> Value;
const bool FromInherit;
AttributeKind Kind;

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

[[nodiscard]] Node &key() const { return *Key; }

[[nodiscard]] Expr *value() const { return Value.get(); }

[[nodiscard]] bool fromInherit() const { return FromInherit; }
[[nodiscard]] AttributeKind kind() const { return Kind; }

[[nodiscard]] bool fromInherit() const {
return Kind == AttributeKind::InheritFrom || Kind == AttributeKind::Inherit;
}
};

/// \brief Attribute set after deduplication.
Expand Down
7 changes: 4 additions & 3 deletions libnixf/include/nixf/Sema/SemaActions.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class Sema {
std::shared_ptr<Formals> onFormals(LexerCursorRange Range, FormalVector FV);

/// \brief Desugar inherit (expr) a, inherit a, into select, or variable.
static std::shared_ptr<Expr>
static std::pair<std::shared_ptr<Expr>, Attribute::AttributeKind>
desugarInheritExpr(std::shared_ptr<AttrName> Name, std::shared_ptr<Expr> E);

void dupAttr(std::string Name, LexerCursorRange Range, LexerCursorRange Prev);
Expand All @@ -72,7 +72,7 @@ class Sema {

/// \note Name must not be null
void insertAttr(SemaAttrs &SA, std::shared_ptr<AttrName> Name,
std::shared_ptr<Expr> E, bool IsInherit);
std::shared_ptr<Expr> E, Attribute::AttributeKind Kind);

/// Select into \p Attr the attribute specified by \p Path, or create one if
/// not exists, until reached the inner-most attr. Similar to `mkdir -p`.
Expand All @@ -85,7 +85,8 @@ class Sema {
void addAttr(SemaAttrs &Attr, const AttrPath &Path, std::shared_ptr<Expr> E);

void lowerInheritName(SemaAttrs &SA, std::shared_ptr<AttrName> Name,
std::shared_ptr<Expr> E);
std::shared_ptr<Expr> E,
Attribute::AttributeKind InheritKind);

void lowerInherit(SemaAttrs &Attr, const Inherit &Inherit);

Expand Down
42 changes: 24 additions & 18 deletions libnixf/src/Sema/SemaActions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void Sema::mergeAttrSets(SemaAttrs &XAttrs, const SemaAttrs &YAttrs) {
}

void Sema::insertAttr(SemaAttrs &SA, std::shared_ptr<AttrName> Name,
std::shared_ptr<Expr> E, bool IsInherit) {
std::shared_ptr<Expr> E, Attribute::AttributeKind Kind) {
// In this function we accept nullptr "E".
//
// e.g. { a = ; }
Expand All @@ -76,7 +76,7 @@ void Sema::insertAttr(SemaAttrs &SA, std::shared_ptr<AttrName> Name,
assert(Name);
if (!Name->isStatic()) {
if (E)
SA.Dynamic.emplace_back(std::move(Name), std::move(E), IsInherit);
SA.Dynamic.emplace_back(std::move(Name), std::move(E), Kind);
return;
}
auto &Attrs = SA.Static;
Expand All @@ -97,8 +97,7 @@ void Sema::insertAttr(SemaAttrs &SA, std::shared_ptr<AttrName> Name,
}
if (!E)
return;
Attrs.insert(
{StaticName, Attribute(std::move(Name), std::move(E), IsInherit)});
Attrs.insert({StaticName, Attribute(std::move(Name), std::move(E), Kind)});
}

SemaAttrs *
Expand Down Expand Up @@ -135,8 +134,9 @@ Sema::selectOrCreate(SemaAttrs &SA,
auto NewNested = std::make_shared<ExprAttrs>(
Name->range(), nullptr, nullptr, SemaAttrs(/*Recursive=*/nullptr));
Inner = &NewNested->SA;
StaticAttrs.insert({StaticName, Attribute(Name, std::move(NewNested),
/*FromInherit=*/false)});
StaticAttrs.insert(
{StaticName, Attribute(Name, std::move(NewNested),
Attribute::AttributeKind::Plain)});
}
} else {
// Create a dynamic attribute.
Expand All @@ -146,7 +146,7 @@ Sema::selectOrCreate(SemaAttrs &SA,
Inner = &NewNested->SA;
DynamicAttrs.emplace_back(Name,
std::shared_ptr<Expr>(std::move(NewNested)),
/*FromInherit=*/false);
Attribute::AttributeKind::Plain);
}
}
return Inner;
Expand All @@ -163,7 +163,8 @@ void Sema::addAttr(SemaAttrs &Attr, const AttrPath &Path,
std::shared_ptr<AttrName> Name = Path.names().back();
if (!Name)
return;
insertAttr(*Inner, std::move(Name), std::move(E), false);
insertAttr(*Inner, std::move(Name), std::move(E),
Attribute::AttributeKind::Plain);
}

void Sema::removeFormal(Fix &F, const FormalVector::const_iterator &Rm,
Expand Down Expand Up @@ -279,7 +280,9 @@ std::shared_ptr<Formals> Sema::onFormals(LexerCursorRange Range,
}

void Sema::lowerInheritName(SemaAttrs &SA, std::shared_ptr<AttrName> Name,
std::shared_ptr<Expr> E) {
std::shared_ptr<Expr> E,
Attribute::AttributeKind InheritKind) {
assert(InheritKind != Attribute::AttributeKind::Plain);
if (!Name)
return;
if (!Name->isStatic()) {
Expand All @@ -298,15 +301,15 @@ void Sema::lowerInheritName(SemaAttrs &SA, std::shared_ptr<AttrName> Name,
}
// Insert the attr.
std::string StaticName = Name->staticName();
SA.Static.insert({StaticName, Attribute(std::move(Name), std::move(E),
/*FromInherit=*/true)});
SA.Static.insert(
{StaticName, Attribute(std::move(Name), std::move(E), InheritKind)});
}

void Sema::lowerInherit(SemaAttrs &Attr, const Inherit &Inherit) {
for (const std::shared_ptr<AttrName> &Name : Inherit.names()) {
assert(Name);
std::shared_ptr<Expr> Desugar = desugarInheritExpr(Name, Inherit.expr());
lowerInheritName(Attr, Name, std::move(Desugar));
auto [Desugar, Kind] = desugarInheritExpr(Name, Inherit.expr());
lowerInheritName(Attr, Name, std::move(Desugar), Kind);
}
}

Expand All @@ -330,17 +333,20 @@ void Sema::lowerBinds(SemaAttrs &SA, const Binds &B) {
}
}

std::shared_ptr<Expr> Sema::desugarInheritExpr(std::shared_ptr<AttrName> Name,
std::shared_ptr<Expr> E) {
std::pair<std::shared_ptr<Expr>, Attribute::AttributeKind>
Sema::desugarInheritExpr(std::shared_ptr<AttrName> Name,
std::shared_ptr<Expr> E) {
auto Range = Name->range();
if (!E)
return std::make_shared<ExprVar>(Range, Name->id());
return {std::make_shared<ExprVar>(Range, Name->id()),
Attribute::AttributeKind::Inherit};

auto Path = std::make_shared<AttrPath>(
Range, std::vector<std::shared_ptr<AttrName>>{std::move(Name)},
std::vector<std::shared_ptr<Dot>>{});
return std::make_shared<ExprSelect>(Range, std::move(E), std::move(Path),
nullptr);
return {std::make_shared<ExprSelect>(Range, std::move(E), std::move(Path),
nullptr),
Attribute::AttributeKind::InheritFrom};
}

std::shared_ptr<ExprAttrs> Sema::onExprAttrs(LexerCursorRange Range,
Expand Down
8 changes: 7 additions & 1 deletion libnixf/src/Sema/VariableLookup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,13 @@ std::shared_ptr<EnvNode> VariableLookupAnalysis::dfsAttrs(
for (const auto &[_, Attr] : SA.staticAttrs()) {
if (!Attr.value())
continue;
dfs(*Attr.value(), NewEnv);
if (Attr.kind() == Attribute::AttributeKind::Plain ||
Attr.kind() == Attribute::AttributeKind::InheritFrom) {
dfs(*Attr.value(), NewEnv);
} else {
assert(Attr.kind() == Attribute::AttributeKind::Inherit);
dfs(*Attr.value(), Env);
}
}

dfsDynamicAttrs(SA.dynamicAttrs(), NewEnv);
Expand Down
27 changes: 14 additions & 13 deletions libnixf/test/Sema/SemaActions.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,11 +110,12 @@ TEST_F(SemaActionTest, insertAttrDup) {
{"a", Attribute(
/*Key=*/Name,
/*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false)});
Attribute::AttributeKind::Plain)});

SemaAttrs A(std::move(Attrs), {}, nullptr);
std::shared_ptr<ExprInt> E(new ExprInt{{}, 1});
L.insertAttr(A, std::move(Name), std::move(E), false); // Duplicated
L.insertAttr(A, std::move(Name), std::move(E),
Attribute::AttributeKind::Plain); // Duplicated

ASSERT_EQ(A.staticAttrs().size(), 1);
ASSERT_EQ(Diags.size(), 1);
Expand All @@ -126,7 +127,7 @@ TEST_F(SemaActionTest, insertAttrOK) {
// Check we can detect duplicated attr.
std::shared_ptr<ExprInt> E(new ExprInt{{}, 1});
L.insertAttr(SA, std::move(Name), std::move(E),
/*IsInherit=*/false); // Duplicated
Attribute::AttributeKind::Plain); // Duplicated
ASSERT_EQ(SA.staticAttrs().size(), 1);
ASSERT_EQ(Diags.size(), 0);
}
Expand All @@ -136,16 +137,15 @@ TEST_F(SemaActionTest, insertAttrNullptr) {
auto Name = getStaticName("a");
std::shared_ptr<ExprInt> E(new ExprInt{{}, 1});
L.insertAttr(SA, std::move(Name), std::move(E),
/*IsInherit=*/false); // Duplicated
Attribute::AttributeKind::Plain); // Duplicated
ASSERT_EQ(SA.staticAttrs().size(), 1);
ASSERT_EQ(Diags.size(), 0);
}

TEST_F(SemaActionTest, insertAttrNullptr2) {
SemaAttrs SA(nullptr);
auto Name = getDynamicName();
L.insertAttr(SA, std::move(Name), nullptr,
/*IsInherit=*/false);
L.insertAttr(SA, std::move(Name), nullptr, Attribute::AttributeKind::Plain);
ASSERT_EQ(SA.staticAttrs().size(), 0);
ASSERT_EQ(SA.dynamicAttrs().size(), 0);
ASSERT_EQ(Diags.size(), 0);
Expand All @@ -155,7 +155,8 @@ TEST_F(SemaActionTest, inheritName) {
SemaAttrs Attr(nullptr);
auto Name = getStaticName("a");

L.lowerInheritName(Attr, std::move(Name), nullptr);
L.lowerInheritName(Attr, std::move(Name), nullptr,
Attribute::AttributeKind::Inherit);
ASSERT_EQ(Attr.staticAttrs().size(), 1);
ASSERT_EQ(Diags.size(), 0);
}
Expand All @@ -164,7 +165,7 @@ TEST_F(SemaActionTest, inheritNameNullptr) {
SemaAttrs Attr(nullptr);
auto Name = getStaticName("a");

L.lowerInheritName(Attr, nullptr, nullptr);
L.lowerInheritName(Attr, nullptr, nullptr, Attribute::AttributeKind::Inherit);
ASSERT_EQ(Attr.staticAttrs().size(), 0);
ASSERT_EQ(Diags.size(), 0);
}
Expand All @@ -174,7 +175,7 @@ TEST_F(SemaActionTest, inheritNameDynamic) {
auto Name = getDynamicName(
{LexerCursor::unsafeCreate(0, 0, 1), LexerCursor::unsafeCreate(0, 1, 2)});

L.lowerInheritName(Attr, Name, nullptr);
L.lowerInheritName(Attr, Name, nullptr, Attribute::AttributeKind::Inherit);
ASSERT_EQ(Attr.staticAttrs().size(), 0);
ASSERT_EQ(Attr.dynamicAttrs().size(), 0);
ASSERT_EQ(Diags.size(), 1);
Expand All @@ -198,10 +199,10 @@ TEST_F(SemaActionTest, inheritNameDuplicated) {
{"a", Attribute(
/*Key=*/Name,
/*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false)});
Attribute::AttributeKind::Plain)});

SemaAttrs SA(std::move(Attrs), {}, nullptr);
L.lowerInheritName(SA, Name, nullptr);
L.lowerInheritName(SA, Name, nullptr, Attribute::AttributeKind::Inherit);
ASSERT_EQ(SA.staticAttrs().size(), 1);
ASSERT_EQ(Diags.size(), 1);
}
Expand All @@ -222,13 +223,13 @@ TEST_F(SemaActionTest, mergeAttrSets) {
{"a", Attribute(
/*Key=*/XName,
/*Value=*/std::make_shared<ExprInt>(LexerCursorRange{}, 1),
/*FromInherit=*/false)});
Attribute::AttributeKind::Plain)});

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

SemaAttrs XA(XAttrs, {}, nullptr);
SemaAttrs YA(YAttrs, {}, nullptr);
Expand Down
18 changes: 18 additions & 0 deletions libnixf/test/Sema/VariableLookup.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -290,4 +290,22 @@ TEST_F(VLATest, EmptyLetIn) {
ASSERT_EQ(Diags.size(), 0);
}

TEST_F(VLATest, Issue513) {
// #513
const char *Src = R"(
rec {
a = 1;
b = rec {
inherit a;
};
})";

std::shared_ptr<Node> AST = parse(Src, Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_EQ(Diags.size(), 1);
ASSERT_EQ(Diags[0].range().lCur().line(), 3);
}

} // namespace

0 comments on commit 2d37fb2

Please sign in to comment.