Skip to content

Commit

Permalink
Add lint rule for UnusedTaskIntent (#26005)
Browse files Browse the repository at this point in the history
Adds a new lint rule UnusedTaskIntent. This is similar to UnusedFormal
and UnusedLoopIndex and warns for unused task intents like the following

```chapel
var a = 1;
begin with (ref a) { }
```

Since `a` is not used, it can be safely removed.

Note that the following is not warning for, since task private variables
could have side effects

```chapel
begin with (var x = ...) { }
```

This PR also fixes an issue where `@chplcheck.ignore` was being added to
AstNode's that did not support attributes for the `IncorrectIndentation`
rule

Future work: Improve the naming of sub-location method names like
`block_header` and `header_location`

[Reviewed by @DanilaFe]
  • Loading branch information
jabraham17 authored Oct 1, 2024
2 parents 1453dbf + 844d700 commit 0c094d6
Show file tree
Hide file tree
Showing 12 changed files with 207 additions and 40 deletions.
10 changes: 5 additions & 5 deletions test/chplcheck/DoKeywordAndBlock.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ module DoKeywordAndBlock {
var A: [1..10] int;

@chplcheck.ignore("UnusedLoopIndex")
forall i in 1..10 with (ref A) do {}
forall i in 1..10 with (ref A) do { A; }

@chplcheck.ignore("UnusedLoopIndex")
forall i in 1..10 with (ref A) do
{}
{ A; }

@chplcheck.ignore("UnusedLoopIndex")
forall i in 1..10 with (ref A) do {

A;
}

@chplcheck.ignore("UnusedLoopIndex")
Expand Down Expand Up @@ -49,8 +49,8 @@ module DoKeywordAndBlock {
do {}
for 1..10 do{
}
forall 1..10 with (ref A)do{}
forall 1..10 with (ref A)do {}
forall 1..10 with (ref A)do{ A; }
forall 1..10 with (ref A)do { A; }

on Locales[0] do {}

Expand Down
10 changes: 5 additions & 5 deletions test/chplcheck/DoKeywordAndBlock.good-fixit
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,15 @@ module DoKeywordAndBlock {
var A: [1..10] int;

@chplcheck.ignore("UnusedLoopIndex")
forall i in 1..10 with (ref A) {}
forall i in 1..10 with (ref A) { A; }

@chplcheck.ignore("UnusedLoopIndex")
forall i in 1..10 with (ref A)
{}
{ A; }

@chplcheck.ignore("UnusedLoopIndex")
forall i in 1..10 with (ref A) {

A;
}

@chplcheck.ignore("UnusedLoopIndex")
Expand Down Expand Up @@ -49,8 +49,8 @@ module DoKeywordAndBlock {
{}
for 1..10 {
}
forall 1..10 with (ref A) {}
forall 1..10 with (ref A) {}
forall 1..10 with (ref A) { A; }
forall 1..10 with (ref A) { A; }

on Locales[0] {}

Expand Down
12 changes: 12 additions & 0 deletions test/chplcheck/IncorrectIndentation.good
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,16 @@ IncorrectIndentation.chpl:137: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:142: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:146: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:151: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:156: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:162: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:164: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:167: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:168: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:171: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:173: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:176: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:177: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:180: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:182: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:196: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:197: node violates rule IncorrectIndentation
Expand All @@ -36,16 +42,22 @@ IncorrectIndentation.chpl:202: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:207: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:211: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:217: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:220: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:222: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:223: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:226: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:228: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:229: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:232: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:233: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:234: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:237: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:238: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:239: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:242: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:243: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:243: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:246: node violates rule UnusedTaskIntent
IncorrectIndentation.chpl:247: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:248: node violates rule IncorrectIndentation
IncorrectIndentation.chpl:249: node violates rule IncorrectIndentation
Expand Down
32 changes: 12 additions & 20 deletions test/chplcheck/IncorrectIndentation.good-fixit
Original file line number Diff line number Diff line change
Expand Up @@ -107,13 +107,11 @@ module IncorrectIndentation {
writeln("??");
}

@chplcheck.ignore("IncorrectIndentation")
on here
{
writeln("hi");
}

@chplcheck.ignore("IncorrectIndentation")
on here {
writeln("hi");
}
Expand All @@ -139,13 +137,11 @@ module IncorrectIndentation {
writeln("??");
}

@chplcheck.ignore("IncorrectIndentation")
begin
{
writeln("hi");
}

@chplcheck.ignore("IncorrectIndentation")
begin {
writeln("hi");
}
Expand All @@ -167,33 +163,31 @@ module IncorrectIndentation {

var dummy: int;

begin with (ref dummy)
begin
{
writeln("hi");
writeln("??");
}

@chplcheck.ignore("IncorrectIndentation")
begin with (ref dummy)
begin
{
writeln("hi");
}

@chplcheck.ignore("IncorrectIndentation")
begin with (ref dummy) {
begin {
writeln("hi");
}

begin with (ref dummy) {
begin {
writeln("hi");
writeln("hi");
}

begin with (ref dummy) {
begin {
writeln("hi"); writeln("hi");
}

begin with (ref dummy) {
begin {
@chplcheck.ignore("IncorrectIndentation")
for 1..10 do
writeln("hi");
Expand All @@ -208,14 +202,12 @@ module IncorrectIndentation {
writeln("??");
}

@chplcheck.ignore("IncorrectIndentation")
cobegin
{
writeln("hi");
writeln("hi");
}

@chplcheck.ignore("IncorrectIndentation")
cobegin {
writeln("hi");
writeln("hi");
Expand All @@ -237,33 +229,33 @@ module IncorrectIndentation {
writeln("hi");
}

cobegin with (ref dummy)
cobegin
{
writeln("hi");
writeln("??");
}

cobegin with (ref dummy)
cobegin
{
writeln("hi");
writeln("hi");
}

cobegin with (ref dummy) {
cobegin {
writeln("hi");
writeln("hi");
}

cobegin with (ref dummy) {
cobegin {
writeln("hi");
writeln("hi");
}

cobegin with (ref dummy) {
cobegin {
writeln("hi"); writeln("hi");
}

cobegin with (ref dummy) {
cobegin {
writeln("hi");
@chplcheck.ignore("IncorrectIndentation")
for 1..10 do
Expand Down
8 changes: 4 additions & 4 deletions test/chplcheck/SimpleDomainAsRange.chpl
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,19 @@ module SimpleDomainAsRange {

@chplcheck.ignore("UnusedLoopIndex")
forall i in {1..#10 by 2 align 2} with (ref A) {

A;
}

forall {1..10} with (ref A) {

A;
}

forall {1..<10} with (ref A) {

A;
}

foreach {1..#10 by 2 align 2} with (ref A) {

A;
}

foreach {1..10, 1..10 by 2} {}
Expand Down
8 changes: 4 additions & 4 deletions test/chplcheck/SimpleDomainAsRange.good-fixit
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,19 @@ module SimpleDomainAsRange {

@chplcheck.ignore("UnusedLoopIndex")
forall i in 1..#10 by 2 align 2 with (ref A) {

A;
}

forall 1..10 with (ref A) {

A;
}

forall 1..<10 with (ref A) {

A;
}

foreach 1..#10 by 2 align 2 with (ref A) {

A;
}

foreach {1..10, 1..10 by 2} {}
Expand Down
35 changes: 35 additions & 0 deletions test/chplcheck/UnusedTaskIntent.chpl
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
module UnusedTaskIntent {
var A: [1..10] int = 10;
var B: int = 1;

forall 1..10 with (var x: int) {}

forall 1..10 with (var x = B) {}

foreach 1..10
with (ref A,
in B) {}

coforall 1..10 with (+ reduce A) {}

begin with (ref A,
+ reduce B) {}

begin with
(ref A) {}

cobegin with
(const in A) { }

[1..10 with(ref A)] { ; }

[1..10 with (in A,
const in B)] { ; }

@chplcheck.ignore("UnusedTaskIntent")
[1..10 with(ref A)] { ; }

forall 1..10 with (const A) { }
forall 1..10 with (const x: int) { }
forall 1..10 with (const x = 1) { }
}
13 changes: 13 additions & 0 deletions test/chplcheck/UnusedTaskIntent.good
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
UnusedTaskIntent.chpl:21: warning: cobegin has no effect if it contains fewer than 2 statements
UnusedTaskIntent.chpl:10: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:11: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:13: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:15: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:16: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:19: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:22: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:24: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:26: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:27: node violates rule UnusedTaskIntent
UnusedTaskIntent.chpl:32: node violates rule UnusedTaskIntent
[Success matching fixit for UnusedTaskIntent]
35 changes: 35 additions & 0 deletions test/chplcheck/UnusedTaskIntent.good-fixit
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
module UnusedTaskIntent {
var A: [1..10] int = 10;
var B: int = 1;

forall 1..10 with (var x: int) {}

forall 1..10 with (var x = B) {}

@chplcheck.ignore("UnusedTaskIntent")
foreach 1..10
with (ref A,
in B) {}

coforall 1..10 {}

begin with (ref A,
+ reduce B) {}

begin {}

cobegin { }

[1..10] { ; }

@chplcheck.ignore("UnusedTaskIntent")
[1..10 with (in A,
const in B)] { ; }

@chplcheck.ignore("UnusedTaskIntent")
[1..10 with(ref A)] { ; }

forall 1..10 { }
forall 1..10 with (const x: int) { }
forall 1..10 with (const x = 1) { }
}
1 change: 1 addition & 0 deletions test/chplcheck/activerules.good
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ Active rules:
SimpleDomainAsRange Warn for simple domains in loops that can be ranges.
UnusedFormal Warn for unused formals in functions.
UnusedLoopIndex Warn for unused index variables in loops.
UnusedTaskIntent Warn for unused task intents in functions.
UnusedTupleUnpack Warn for unused tuple unpacking, such as '(_, _)'.
1 change: 1 addition & 0 deletions test/chplcheck/allrules.good
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,6 @@ Available rules (default rules marked with *):
* SimpleDomainAsRange Warn for simple domains in loops that can be ranges.
* UnusedFormal Warn for unused formals in functions.
* UnusedLoopIndex Warn for unused index variables in loops.
* UnusedTaskIntent Warn for unused task intents in functions.
* UnusedTupleUnpack Warn for unused tuple unpacking, such as '(_, _)'.
UseExplicitModules Warn for code that relies on auto-inserted implicit modules.
Loading

0 comments on commit 0c094d6

Please sign in to comment.