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.

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

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

Merged
merged 8 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 9 additions & 4 deletions libnixf/include/nixf/Sema/VariableLookup.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,16 @@ class Definition {
/// \brief From ambda arg e.g. a: a + 1
DS_LambdaArg,

/// \brief From lambda formal, e.g. { a }: a + 1
DS_LambdaFormal,
/// \brief From lambda (noarg) formal, e.g. { a }: a + 1
DS_LambdaNoArg_Formal,

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

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

/// \brief From recursive attribute set. e.g. rec { }
DS_Rec,
Expand Down
24 changes: 21 additions & 3 deletions libnixf/src/Basic/diagnostic.py
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,28 @@ class Diagnostic(TypedDict):
"message": "undefined variable `{}`",
},
{
"sname": "sema-def-not-used",
"cname": "DefinitionNotUsed",
"sname": "sema-unused-def-let",
"cname": "UnusedDefLet",
"severity": "Warning",
"message": "definition `{}` in let-expression is not used",
},
{
"sname": "sema-unused-def-lambda-noarg-formal",
"cname": "UnusedDefLambdaNoArg_Formal",
"severity": "Warning",
"message": "attribute `{}` of argument is not used",
},
{
"sname": "sema-unused-def-lambda-witharg-formal",
"cname": "UnusedDefLambdaWithArg_Formal",
"severity": "Warning",
"message": "argument `{}` in `@`-pattern is not used",
},
{
"sname": "sema-unused-def-lambda-witharg-arg",
"cname": "UnusedDefLambdaWithArg_Arg",
"severity": "Hint",
"message": "definition `{}` is not used",
"message": "attribute `{}` of `@`-pattern argument is not used, but may be referenced from the argument",
},
{
"sname": "sema-extra-rec",
Expand Down
45 changes: 36 additions & 9 deletions libnixf/src/Sema/VariableLookup.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,22 @@ 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_LambdaNoArg_Formal:
return Diagnostic::DK_UnusedDefLambdaNoArg_Formal;
case Definition::DS_LambdaWithArg_Formal:
return Diagnostic::DK_UnusedDefLambdaWithArg_Formal;
case Definition::DS_LambdaWithArg_Arg:
return Diagnostic::DK_UnusedDefLambdaWithArg_Arg;
default:
assert(false && "liveness diagnostic encountered an unknown source!");
__builtin_unreachable();
}
}();
Diagnostic &D = Diags.emplace_back(Kind, Def->syntax()->range());
D << Name;
D.tag(DiagnosticTag::Faded);
}
Expand Down Expand Up @@ -137,17 +151,30 @@ void VariableLookupAnalysis::dfs(const ExprLambda &Lambda,
} else if (!Arg.formals()->dedup().contains(Arg.id()->name())) {
ToDef.insert_or_assign(Arg.id(),
DBuilder.add(Arg.id()->name(), Arg.id(),
Definition::DS_LambdaArgWithFormal));
Definition::DS_LambdaWithArg_Arg));
}
}

// { 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_LambdaWithArg_Formal
: Definition::DS_LambdaNoArg_Formal;
ToDef.insert_or_assign(Formal->id(),
DBuilder.add(Name, Formal->id(), Source));
}
}

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

Expand Down
24 changes: 17 additions & 7 deletions libnixf/test/Sema/VariableLookup.cpp
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_UnusedDefLambdaNoArg_Formal);
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,9 +177,21 @@ TEST_F(VLATest, LivenessArgWithFormal) {

ASSERT_EQ(Diags.size(), 1);

ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_DefinitionNotUsed);
ASSERT_EQ(Diags[0].kind(), Diagnostic::DK_UnusedDefLambdaWithArg_Arg);
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_UnusedDefLambdaWithArg_Formal);
ASSERT_EQ(Diags[0].range().lCur().column(), 2);
ASSERT_EQ(Diags[0].tags().size(), 1);
ASSERT_EQ(Diags[0].tags()[0], DiagnosticTag::Faded);
}

Expand Down
6 changes: 3 additions & 3 deletions nixd/tools/nixd/test/diagnostic/liveness-formal.md
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-lambda-noarg-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