Skip to content
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

Jump to bottom

libnixf: split sema-unused-def into let, arg and formal #565

Merged
merged 8 commits into from
Aug 2, 2024
Merged

libnixf: split sema-unused-def into let, arg and formal #565

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6e50f77
libnixf: split sema-unused-def into let, arg and formal
Aleksanaa Aug 1, 2024
5c2cbfb
fix undefined diagnostic kind, formal env creating, and the tests
inclyc Aug 2, 2024
cccd515
chatgpt comments
inclyc Aug 2, 2024
58d654f
Update libnixf/src/Basic/diagnostic.py
inclyc Aug 2, 2024
6907c0f
please fmt
inclyc Aug 2, 2024
e21457e
fixup empty line
inclyc Aug 2, 2024
568e16e
remove unreachable code
inclyc Aug 2, 2024
f204a12
rename somewhat
inclyc Aug 2, 2024
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
5 changes: 4 additions & 1 deletion libnixf/include/nixf/Sema/VariableLookup.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,12 @@ class Definition {
/// \brief From lambda formal, e.g. { a }: a + 1
DS_LambdaFormal,

/// \brief From lambda arg with formal, e.g. { foo, bar }@a: a + 1
/// \brief From lambda arg with formal, e.g. `a` in `{ foo }@a: foo + 1`
DS_LambdaArgWithFormal,

/// \brief From lambda formal with arg, e.g. `foo` in `{ foo }@a: foo + 1`
DS_LambdaFormalWithArg,

/// \brief From recursive attribute set. e.g. rec { }
DS_Rec,

Expand Down
30 changes: 27 additions & 3 deletions libnixf/src/Basic/diagnostic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,11 +184,35 @@ class Diagnostic(TypedDict):
"message": "undefined variable `{}`",
},
{
"sname": "sema-def-not-used",
"cname": "DefinitionNotUsed",
"severity": "Hint",
"sname": "sema-unused-def-generic",
inclyc marked this conversation as resolved.
Show resolved Hide resolved
"cname": "UnusedDef",
"severity": "Warning",
"message": "definition `{}` is not used",
},
{
"sname": "sema-unused-def-let",
"cname": "UnusedDefLet",
"severity": "Warning",
"message": "definition `{}` in let-expression is not used",
},
{
"sname": "sema-unused-def-formal",
"cname": "UnusedDefFormal",
"severity": "Warning",
"message": "attribute `{}` of argument is not used",
},
{
"sname": "sema-unused-def-arg-with-formal",
"cname": "UnusedDefArgWithFormal",
"severity": "Warning",
"message": "argument `{}` in `@`-pattern is not used",
},
{
"sname": "sema-unused-def-formal-with-arg",
"cname": "UnusedDefFormalWithArg",
"severity": "Hint",
"message": "attribute `{}` of `@`-pattern argument is not used, but may be referenced from the argument",
},
{
"sname": "sema-extra-rec",
"cname": "ExtraRecursive",
Expand Down
43 changes: 35 additions & 8 deletions libnixf/src/Sema/VariableLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,21 @@ void VariableLookupAnalysis::emitEnvLivenessWarning(
if (!Def->syntax())
continue;
if (Def->uses().empty()) {
Diagnostic &D = Diags.emplace_back(Diagnostic::DK_DefinitionNotUsed,
Def->syntax()->range());
Diagnostic::DiagnosticKind Kind = [&]() {
switch (Def->source()) {
case Definition::DS_Let:
return Diagnostic::DK_UnusedDefLet;
case Definition::DS_LambdaFormal:
return Diagnostic::DK_UnusedDefFormal;
case Definition::DS_LambdaFormalWithArg:
return Diagnostic::DK_UnusedDefFormalWithArg;
case Definition::DS_LambdaArgWithFormal:
return Diagnostic::DK_UnusedDefArgWithFormal;
default:
return Diagnostic::DK_UnusedDef;
inclyc marked this conversation as resolved.
Show resolved Hide resolved
}
}();
Diagnostic &D = Diags.emplace_back(Kind, Def->syntax()->range());
D << Name;
D.tag(DiagnosticTag::Faded);
}
Expand Down Expand Up @@ -128,6 +141,7 @@ void VariableLookupAnalysis::dfs(const ExprLambda &Lambda,

// foo: body
// ^~~<------- add function argument.

inclyc marked this conversation as resolved.
Show resolved Hide resolved
if (Arg.id()) {
if (!Arg.formals()) {
ToDef.insert_or_assign(Arg.id(), DBuilder.add(Arg.id()->name(), Arg.id(),
Expand All @@ -142,12 +156,25 @@ void VariableLookupAnalysis::dfs(const ExprLambda &Lambda,
}

// { foo, bar, ... } : body
/// ^~~~~~~~~<-------------- add function formals.
if (Arg.formals())
for (const auto &[Name, Formal] : Arg.formals()->dedup())
ToDef.insert_or_assign(
Formal->id(),
DBuilder.add(Name, Formal->id(), Definition::DS_LambdaFormal));
// ^~~~~~~~~<-------------- add function formals.

// This section differentiates between formal parameters with an argument and
// without. Example:
//
// { foo }@arg : use arg
//
// In this case, the definition of `foo` is not used directly; however, it
// might be accessed via arg.foo. Therefore, the severity of an unused formal
// parameter is reduced in this scenario.
if (Arg.formals()) {
for (const auto &[Name, Formal] : Arg.formals()->dedup()) {
Definition::DefinitionSource Source =
Arg.id() ? Definition::DS_LambdaFormalWithArg
: Definition::DS_LambdaFormal;
ToDef.insert_or_assign(Formal->id(),
DBuilder.add(Name, Formal->id(), Source));
}
}

auto NewEnv = std::make_shared<EnvNode>(Env, DBuilder.finish(), &Lambda);

Expand Down
25 changes: 18 additions & 7 deletions libnixf/test/Sema/VariableLookup.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,29 +135,27 @@ TEST_F(VLATest, LivenessRec) {
ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_ExtraRecursive);
}

TEST_F(VLATest, LivenessArg) {
TEST_F(VLATest, LivenessFormal) {
std::shared_ptr<Node> AST = parse("{ foo }: 1", Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_EQ(Diags.size(), 1);

ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed);
ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefFormal);
ASSERT_EQ(Diags[0].tags().size(), 1);
ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded);
}

TEST_F(VLATest, LivenessNested) {
TEST_F(VLATest, LivenessLet) {
std::shared_ptr<Node> AST = parse("let y = 1; in x: y: x + y", Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_EQ(Diags.size(), 1);

ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed);
ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLet);
ASSERT_EQ(Diags[0].range().lCur().column(), 4);
ASSERT_EQ(Diags[0].tags().size(), 1);
ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded);
}

TEST_F(VLATest, LivenessDupSymbol) {
Expand All @@ -179,12 +177,25 @@ TEST_F(VLATest, LivenessArgWithFormal) {

ASSERT_EQ(Diags.size(), 1);

ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed);
ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefArgWithFormal);
ASSERT_EQ(Diags[0].range().lCur().column(), 8);
ASSERT_EQ(Diags[0].tags().size(), 1);
}

TEST_F(VLATest, LivenessFormalWithArg) {
std::shared_ptr<Node> AST = parse("{ foo }@bar: bar", Diags);
VariableLookupAnalysis VLA(Diags);
VLA.runOnAST(*AST);

ASSERT_EQ(Diags.size(), 1);

ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefFormalWithArg);
ASSERT_EQ(Diags[0].range().lCur().column(), 2);
ASSERT_EQ(Diags[0].tags().size(), 1);
ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded);
}


TEST_F(VLATest, ToDefAttrs) {
std::shared_ptr<Node> AST = parse("rec { x = 1; y = x; z = x; }", Diags);
VariableLookupAnalysis VLA(Diags);
Expand Down
6 changes: 3 additions & 3 deletions nixd/tools/nixd/test/diagnostic/liveness-formal.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@
```
CHECK: "diagnostics": [
CHECK-NEXT: {
CHECK-NEXT: "code": "sema-def-not-used",
CHECK-NEXT: "message": "definition `y` is not used",
CHECK-NEXT: "code": "sema-unused-def-formal",
CHECK-NEXT: "message": "attribute `y` of argument is not used",
CHECK-NEXT: "range": {
CHECK-NEXT: "end": {
CHECK-NEXT: "character": 5,
Expand All @@ -51,7 +51,7 @@ CHECK-NEXT: "line": 0
CHECK-NEXT: }
CHECK-NEXT: },
CHECK-NEXT: "relatedInformation": [],
CHECK-NEXT: "severity": 4,
CHECK-NEXT: "severity": 2,
CHECK-NEXT: "source": "nixf",
CHECK-NEXT: "tags": [
CHECK-NEXT: 1
Expand Down
Loading