From 68ff6bcbaeccfd851b16c212b9d7cecd518e662f Mon Sep 17 00:00:00 2001 From: Yingchi Long Date: Thu, 21 Sep 2023 01:12:49 +0800 Subject: [PATCH] nixd/Sema: add lowering support for attrs --- nixd/include/nixd/Sema/Lowering.h | 19 +- nixd/lib/Sema/Lowering.cpp | 207 +++++++++++++++--- .../test/lowering-attrset-dynamic.nix | 7 + .../test/lowering-attrset-merge-inherit.nix | 7 + .../test/lowering-attrset-merging-2.nix | 10 + .../nixd-lint/test/lowering-attrset-rec-2.nix | 14 ++ .../nixd-lint/test/lowering-attrset-rec-3.nix | 15 ++ .../nixd-lint/test/lowering-attrset-rec.nix | 13 ++ .../test/lowering-attrset-select-merging.nix | 12 + 9 files changed, 270 insertions(+), 34 deletions(-) create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-dynamic.nix create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-merge-inherit.nix create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-merging-2.nix create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-rec-2.nix create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-rec-3.nix create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-rec.nix create mode 100644 nixd/tools/nixd-lint/test/lowering-attrset-select-merging.nix diff --git a/nixd/include/nixd/Sema/Lowering.h b/nixd/include/nixd/Sema/Lowering.h index 9cdc367d3..f6deeb349 100644 --- a/nixd/include/nixd/Sema/Lowering.h +++ b/nixd/include/nixd/Sema/Lowering.h @@ -18,14 +18,28 @@ class ExprAttrsBuilder { Lowering &LW; nix::ExprAttrs *Result; + bool Recursive; + + RangeIdx Range; + + /// Nested attributes, we create a new builder for them, and collapse the map + /// while finishing + std::map> Nested; + + std::map> DynamicNested; + // Use a map to detect duplicated fields // Not using the map inside nix::ExprAttrs because we want to preserve the // range - std::map Fields; + std::map Fields; public: - ExprAttrsBuilder(Lowering &LW); + ExprAttrsBuilder(Lowering &LW, bool Recursive, RangeIdx Range); + void addAttr(const syntax::Node *Attr, const syntax::Node *Body, + bool Recursive); void addAttribute(const syntax::Attribute &A); + void addBinds(const syntax::Binds &Binds); + void addAttrSet(const syntax::AttrSet &AS); void addInherited(const syntax::InheritedAttribute &IA); nix::ExprAttrs *finish(); }; @@ -40,7 +54,6 @@ struct Lowering { nix::ExprLambda *lowerFunction(const syntax::Function *Fn); nix::Formal lowerFormal(const syntax::Formal &Formal); nix::AttrPath lowerAttrPath(const syntax::AttrPath &Path); - nix::ExprAttrs *lowerBinds(const syntax::Binds &Binds); }; } // namespace nixd diff --git a/nixd/lib/Sema/Lowering.cpp b/nixd/lib/Sema/Lowering.cpp index 9dc197dc6..d4bf9fa84 100644 --- a/nixd/lib/Sema/Lowering.cpp +++ b/nixd/lib/Sema/Lowering.cpp @@ -2,14 +2,19 @@ #include "nixd/Sema/EvalContext.h" #include "nixd/Syntax/Diagnostic.h" #include "nixd/Syntax/Nodes.h" +#include "nixd/Syntax/Range.h" #include #include +#include + namespace nixd { +using syntax::Diagnostic; using syntax::Node; +using syntax::Note; nix::Formal Lowering::lowerFormal(const syntax::Formal &Formal) { auto *Def = Lowering::lower(Formal.Default); @@ -129,13 +134,171 @@ static syntax::Diagnostic mkAttrDupDiag(std::string Name, RangeIdx Range, return Diag; } -ExprAttrsBuilder::ExprAttrsBuilder(Lowering &LW) : LW(LW) { +ExprAttrsBuilder::ExprAttrsBuilder(Lowering &LW, bool Recursive, RangeIdx Range) + : LW(LW), Recursive(Recursive), Range(Range) { Result = LW.Ctx.Pool.record(new nix::ExprAttrs); } -nix::ExprAttrs *ExprAttrsBuilder::finish() { return Result; } +nix::ExprAttrs *ExprAttrsBuilder::finish() { + Result->recursive = Recursive; + for (const auto &[Sym, Builder] : Nested) { + auto *NestedAttr = Builder->finish(); + nix::ExprAttrs::AttrDef Def(NestedAttr, NestedAttr->pos, false); + Result->attrs.insert({Sym, Def}); + } + for (const auto &[Node, Builder] : DynamicNested) { + nix::Expr *Name = LW.lower(Node); + nix::ExprAttrs *Value = Builder->finish(); + nix::ExprAttrs::DynamicAttrDef DDef(Name, Value, Value->pos); + Result->dynamicAttrs.emplace_back(DDef); + } + return Result; +} + +void ExprAttrsBuilder::addBinds(const syntax::Binds &Binds) { + for (Node *Attr : Binds.Attributes) { + switch (Attr->getKind()) { + case Node::NK_Attribute: + addAttribute(*dynamic_cast(Attr)); + break; + case Node::NK_InheritedAttribute: + addInherited(*dynamic_cast(Attr)); + break; + default: + llvm_unreachable("attr in binds must be either A or IA!"); + } + } +} + +static Diagnostic mkRecDiag(RangeIdx ThisRange, RangeIdx MergingRange, + bool ThisRec, bool MergningRec) { + assert(ThisRec != MergningRec); + + Diagnostic Diag; + Diag.Kind = Diagnostic::Warning; + Diag.Msg = "merging two attributes with different `rec` modifiers, the " + "latter will be implicitly ignored"; + Diag.Range = MergingRange; + + Note This; + This.Range = ThisRange; + This.Msg = llvm::formatv("this attribute set is{0} recursive", + ThisRec ? "" : " not"); + Diag.Notes.emplace_back(std::move(This)); + + Note Merging; + Merging.Range = MergingRange; + Merging.Msg = + llvm::formatv("while this attribute set is marked as {0}recursive, it " + "will be considered as {1}recursive", + MergningRec ? "" : "non-", ThisRec ? "" : "non-"); + Diag.Notes.emplace_back(std::move(Merging)); + return Diag; +} -void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {} +void ExprAttrsBuilder::addAttrSet(const syntax::AttrSet &AS) { + if (AS.Recursive != Recursive) { + auto Diag = mkRecDiag(Range, AS.Range, Recursive, AS.Recursive); + LW.Diags.emplace_back(std::move(Diag)); + } + assert(AS.AttrBinds && "binds should not be empty! parser error?"); + addBinds(*AS.AttrBinds); +} + +void ExprAttrsBuilder::addAttr(const syntax::Node *Attr, + const syntax::Node *Body, bool Recursive) { + if (Recursive != this->Recursive) { + auto Diag = mkRecDiag(Range, Attr->Range, this->Recursive, Recursive); + LW.Diags.emplace_back(std::move(Diag)); + } + switch (Attr->getKind()) { + case Node::NK_Identifier: { + const auto *ID = dynamic_cast(Attr); + nix::Symbol Sym = ID->Symbol; + if (Fields.contains(Sym)) { + // duplicated, but if they are two attrset, we can try to merge them. + if (Body->getKind() != Node::NK_AttrSet || !Nested.contains(Sym)) { + // the body is not an attribute set, report error. + auto Diag = + mkAttrDupDiag(LW.STable[Sym], ID->Range, Fields[Sym]->Range); + LW.Diags.emplace_back(std::move(Diag)); + return; + } + const auto *BodyAttrSet = dynamic_cast(Body); + Nested[Sym]->addAttrSet(*BodyAttrSet); + } else { + Fields[Sym] = Attr; + if (Body->getKind() == Node::NK_AttrSet) { + // If the body is an attrset, we want to defer it's lowering process, + // because we have chance to merge it latter + const auto *BodyAttrSet = dynamic_cast(Body); + Nested[Sym] = std::make_unique( + LW, BodyAttrSet->Recursive, BodyAttrSet->Range); + Nested[Sym]->addBinds(*BodyAttrSet->AttrBinds); + } else { + nix::ExprAttrs::AttrDef Def(LW.lower(Body), Attr->Range.Begin); + Result->attrs.insert({Sym, Def}); + } + } + break; + } + default: { + nix::Expr *NameExpr = LW.lower(Attr); + nix::Expr *ValueExpr = LW.lower(Body); + nix::ExprAttrs::DynamicAttrDef DDef(NameExpr, ValueExpr, Attr->Range.Begin); + Result->dynamicAttrs.emplace_back(DDef); + } + } +} + +void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) { + assert(!A.Path->Names.empty() && "attrpath should not be empty!"); + + // The targeting attribute builder we are actually inserting + // we will select the attrpath (`a.b.c`) and assign it to this variable + // left only one nix::Symbol or nix::Expr*, insert to it. + ExprAttrsBuilder *S = this; + + // select attrpath, without the last one. + for (auto I = A.Path->Names.begin(); I + 1 != A.Path->Names.end(); I++) { + Node *Name = *I; + + switch (Name->getKind()) { + case Node::NK_Identifier: { + auto *ID = dynamic_cast(Name); + nix::Symbol Sym = ID->Symbol; + if (S->Fields.contains(Sym)) { + if (!S->Nested.contains(Sym)) { + // contains a field but the body is not an attr, then we cannot + // perform merging. + Diagnostic Diag = + mkAttrDupDiag(LW.STable[Sym], ID->Range, S->Fields[Sym]->Range); + LW.Diags.emplace_back(std::move(Diag)); + return; + } + // select this symbol, and consider merging it. + S = S->Nested[Sym].get(); + } else { + // create a new builder and select to it. + S->Nested[Sym] = std::make_unique( + LW, /*Recursive=*/false, Name->Range); + S->Fields[Sym] = Name; + S = S->Nested[Sym].get(); + } + break; // case Node::NK_Identifier: + } + default: { + DynamicNested[Name] = std::make_unique( + LW, /*Recursive=*/false, Name->Range); + S = DynamicNested[Name].get(); + } + } + } + // Implictly created attribute set, by attrpath selecting, are always + // non-recursive + S->addAttr(A.Path->Names.back(), A.Body, + /*Recursive=*/S != this ? false : this->Recursive); +} void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) { if (IA.InheritedAttrs->Names.empty()) { @@ -198,27 +361,7 @@ void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) { } } -nix::ExprAttrs *Lowering::lowerBinds(const syntax::Binds &Binds) { - ExprAttrsBuilder Builder(*this); - - for (auto *Attr : Binds.Attributes) { - switch (Attr->getKind()) { - case Node::NK_Attribute: { - Builder.addAttribute(*dynamic_cast(Attr)); - break; - } - case Node::NK_InheritedAttribute: { - Builder.addInherited(*dynamic_cast(Attr)); - break; - } - default: - llvm_unreachable("must be A/IA! incorrect parsing rules?"); - } - } - return Builder.finish(); -} - -nix::Expr *Lowering::lower(const nixd::syntax::Node *Root) { +nix::Expr *Lowering::lower(const syntax::Node *Root) { if (!Root) return nullptr; @@ -243,16 +386,18 @@ nix::Expr *Lowering::lower(const nixd::syntax::Node *Root) { Ctx.Pool.record(new nix::ExprWith(With->Range.Begin, Attrs, Body)); return NixWith; } - case Node::NK_Binds: { - const auto *Binds = dynamic_cast(Root); - return lowerBinds(*Binds); - } case Node::NK_AttrSet: { const auto *AttrSet = dynamic_cast(Root); assert(AttrSet->AttrBinds && "null AttrBinds of the AttrSet!"); - nix::ExprAttrs *Binds = lowerBinds(*AttrSet->AttrBinds); - Binds->recursive = AttrSet->Recursive; - return Binds; + ExprAttrsBuilder Builder(*this, AttrSet->Recursive, AttrSet->Range); + Builder.addBinds(*AttrSet->AttrBinds); + return Builder.finish(); + } + case Node::NK_Int: { + const auto *Int = dynamic_cast(Root); + auto *NixInt = new nix::ExprInt(Int->N); + Ctx.Pool.record(NixInt); + return NixInt; } } diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-dynamic.nix b/nixd/tools/nixd-lint/test/lowering-attrset-dynamic.nix new file mode 100644 index 000000000..dbc253406 --- /dev/null +++ b/nixd/tools/nixd-lint/test/lowering-attrset-dynamic.nix @@ -0,0 +1,7 @@ +# RUN: true + +# FIXME: we cannot deal with string_parts currently +# rewrite this test after we can do such thing. +{ + "${bar}" = 1; +} diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-merge-inherit.nix b/nixd/tools/nixd-lint/test/lowering-attrset-merge-inherit.nix new file mode 100644 index 000000000..0217c4aed --- /dev/null +++ b/nixd/tools/nixd-lint/test/lowering-attrset-merge-inherit.nix @@ -0,0 +1,7 @@ +# RUN: nixd-lint %s | FileCheck %s +{ + inherit aa; + + # CHECK: duplicated attr `aa` + aa = 1; +} diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-merging-2.nix b/nixd/tools/nixd-lint/test/lowering-attrset-merging-2.nix new file mode 100644 index 000000000..adcf2d53c --- /dev/null +++ b/nixd/tools/nixd-lint/test/lowering-attrset-merging-2.nix @@ -0,0 +1,10 @@ +# RUN: nixd-lint %s | FileCheck %s +{ + a.bbb = 1; + + # CHECK: error: duplicated attr `b` + a.bbb.c = 2; + + # CHECK: duplicated attr `a` + a = 3; +} diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-rec-2.nix b/nixd/tools/nixd-lint/test/lowering-attrset-rec-2.nix new file mode 100644 index 000000000..8f3f1eb5c --- /dev/null +++ b/nixd/tools/nixd-lint/test/lowering-attrset-rec-2.nix @@ -0,0 +1,14 @@ +# RUN: nixd-lint %s | FileCheck %s +{ + + foo = rec { + x = 1; + y = 2; + }; + + # CHECK: merging two attributes with different `rec` modifiers, the latter will be implicitly ignored + # CHECK: this attribute set is recursive + # CHECK: while this attribute set is marked as non-recursive, it will be considered as recursive + foo.bar = 1; + +} diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-rec-3.nix b/nixd/tools/nixd-lint/test/lowering-attrset-rec-3.nix new file mode 100644 index 000000000..0925de110 --- /dev/null +++ b/nixd/tools/nixd-lint/test/lowering-attrset-rec-3.nix @@ -0,0 +1,15 @@ +# RUN: nixd-lint %s | FileCheck %s +{ + + foo = rec { + x = 1; + y = 2; + }; + + # CHECK: merging two attributes with different `rec` modifiers, the latter will be implicitly ignored + # CHECK: this attribute set is recursive + # CHECK: while this attribute set is marked as non-recursive, it will be considered as recursive + foo = { + bar = 1; + }; +} diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix b/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix new file mode 100644 index 000000000..bea6c7879 --- /dev/null +++ b/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix @@ -0,0 +1,13 @@ +# RUN: nixd-lint %s | FileCheck %s +rec { + foo.bar = 1; + + # CHECK: merging two attributes with different `rec` modifiers, the latter will be implicitly ignored + + # CHECK: this attribute set is not recursive + # CHECK: while this attribute set is marked as recursive, it will be considered as non-recursive + foo = rec { + x = 1; + y = 2; + }; +} diff --git a/nixd/tools/nixd-lint/test/lowering-attrset-select-merging.nix b/nixd/tools/nixd-lint/test/lowering-attrset-select-merging.nix new file mode 100644 index 000000000..1113f408d --- /dev/null +++ b/nixd/tools/nixd-lint/test/lowering-attrset-select-merging.nix @@ -0,0 +1,12 @@ +# RUN: nixd-lint -dump-nix-ast %s | FileCheck %s + +# CHECK: ExprAttrs: { a = { b = 1; c = 2; x = 1; }; } +{ + # CHECK: ExprAttrs: { b = 1; c = 2; x = 1; } + a.b = 1; + a.c = 2; + + a = { + x = 1; + }; +}