From c91aa46853899d29edb8e4b0f13d031a3dc4213f 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 | 12 +- nixd/lib/Sema/Lowering.cpp | 165 +++++++++++++++--- .../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.nix | 10 ++ .../test/lowering-attrset-select-merging.nix | 12 ++ 7 files changed, 193 insertions(+), 30 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.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 8d55a21c5..063d111dd 100644 --- a/nixd/include/nixd/Sema/Lowering.h +++ b/nixd/include/nixd/Sema/Lowering.h @@ -18,14 +18,23 @@ class ExprAttrsBuilder { Lowering &LW; nix::ExprAttrs *Result; + /// 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); + void addAttr(const syntax::Node *Attr, const syntax::Node *Body); 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 +49,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 78a7e0487..8a18672cc 100644 --- a/nixd/lib/Sema/Lowering.cpp +++ b/nixd/lib/Sema/Lowering.cpp @@ -7,8 +7,11 @@ #include +#include + namespace nixd { +using syntax::Diagnostic; using syntax::Node; nix::Formal Lowering::lowerFormal(const syntax::Formal &Formal) { @@ -118,7 +121,7 @@ nix::ExprLambda *Lowering::lowerFunction(const syntax::Function *Fn) { static syntax::Diagnostic mkAttrDupDiag(std::string Name, RangeIdx Range, RangeIdx Prev) { syntax::Diagnostic Diag; - Diag.Kind = syntax::Diagnostic::Warning; + Diag.Kind = syntax::Diagnostic::Error; Diag.Msg = llvm::formatv("duplicated attr `{0}`", Name); Diag.Range = Range; @@ -133,9 +136,126 @@ ExprAttrsBuilder::ExprAttrsBuilder(Lowering &LW) : LW(LW) { Result = LW.Ctx.Pool.record(new nix::ExprAttrs); } -nix::ExprAttrs *ExprAttrsBuilder::finish() { return Result; } +nix::ExprAttrs *ExprAttrsBuilder::finish() { + 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!"); + } + } +} + +void ExprAttrsBuilder::addAttrSet(const syntax::AttrSet &AS) { + if (AS.Recursive) { + Diagnostic Diag; + Diag.Kind = Diagnostic::Warning; + Diag.Msg = "merging nested `rec` attributes, nested `rec` will be " + "implictly ignored"; + Diag.Range = AS.Range; + LW.Diags.emplace_back(std::move(Diag)); + return; + } + assert(AS.AttrBinds && "binds should not be empty! parser error?"); + addBinds(*AS.AttrBinds); +} + +void ExprAttrsBuilder::addAttr(const syntax::Node *Attr, + const syntax::Node *Body) { + 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) { + // 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); + addAttrSet(*BodyAttrSet); + } else { + Fields[Sym] = Attr; + 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!"); -void ExprAttrsBuilder::addAttribute(const syntax::Attribute &A) {} + // 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); + S->Fields[Sym] = Name; + S = S->Nested[Sym].get(); + } + break; // case Node::NK_Identifier: + } + default: { + DynamicNested[Name] = std::make_unique(LW); + S = DynamicNested[Name].get(); + } + } + } + S->addAttr(A.Path->Names.back(), A.Body); +} void ExprAttrsBuilder::addInherited(const syntax::InheritedAttribute &IA) { if (IA.InheritedAttrs->Names.empty()) { @@ -198,27 +318,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; @@ -244,16 +344,25 @@ nix::Expr *Lowering::lower(const nixd::syntax::Node *Root) { return NixWith; } case Node::NK_Binds: { + ExprAttrsBuilder Builder(*this); const auto *Binds = dynamic_cast(Root); - return lowerBinds(*Binds); + Builder.addBinds(*Binds); + return Builder.finish(); } 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); + Builder.addBinds(*AttrSet->AttrBinds); + auto Result = Builder.finish(); + Result->recursive = AttrSet->Recursive; + return Result; } + case Node::NK_Int: + const auto *Int = dynamic_cast(Root); + auto *NixInt = new nix::ExprInt(Int->N); + Ctx.Pool.record(NixInt); + return NixInt; } return nullptr; 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.nix b/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix new file mode 100644 index 000000000..47b554cb1 --- /dev/null +++ b/nixd/tools/nixd-lint/test/lowering-attrset-rec.nix @@ -0,0 +1,10 @@ +# RUN: nixd-lint %s | FileCheck %s +{ + foo.bar = 1; + + # CHECK: merging nested `rec` attributes, nested `rec` will be implictly ignored + foo = rec { + x = 1; + y = x; + }; +} 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..aff2aacf8 --- /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; } + a.b = 1; + a.c = 2; + + a = { + x = 1; + }; +}