diff --git a/test/chplcheck/DoKeywordAndBlock.chpl b/test/chplcheck/DoKeywordAndBlock.chpl index 4d35730d9da..5b8bd4355ad 100644 --- a/test/chplcheck/DoKeywordAndBlock.chpl +++ b/test/chplcheck/DoKeywordAndBlock.chpl @@ -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") @@ -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 {} diff --git a/test/chplcheck/DoKeywordAndBlock.good-fixit b/test/chplcheck/DoKeywordAndBlock.good-fixit index 2f065d15f85..dd905609e66 100644 --- a/test/chplcheck/DoKeywordAndBlock.good-fixit +++ b/test/chplcheck/DoKeywordAndBlock.good-fixit @@ -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") @@ -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] {} diff --git a/test/chplcheck/IncorrectIndentation.good b/test/chplcheck/IncorrectIndentation.good index 0d55d7678bd..8adc50490e8 100644 --- a/test/chplcheck/IncorrectIndentation.good +++ b/test/chplcheck/IncorrectIndentation.good @@ -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 @@ -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 diff --git a/test/chplcheck/IncorrectIndentation.good-fixit b/test/chplcheck/IncorrectIndentation.good-fixit index 74ca4d36c0a..a15ca2d590d 100644 --- a/test/chplcheck/IncorrectIndentation.good-fixit +++ b/test/chplcheck/IncorrectIndentation.good-fixit @@ -107,13 +107,11 @@ module IncorrectIndentation { writeln("??"); } - @chplcheck.ignore("IncorrectIndentation") on here { writeln("hi"); } - @chplcheck.ignore("IncorrectIndentation") on here { writeln("hi"); } @@ -139,13 +137,11 @@ module IncorrectIndentation { writeln("??"); } - @chplcheck.ignore("IncorrectIndentation") begin { writeln("hi"); } - @chplcheck.ignore("IncorrectIndentation") begin { writeln("hi"); } @@ -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"); @@ -208,14 +202,12 @@ module IncorrectIndentation { writeln("??"); } - @chplcheck.ignore("IncorrectIndentation") cobegin { writeln("hi"); writeln("hi"); } - @chplcheck.ignore("IncorrectIndentation") cobegin { writeln("hi"); writeln("hi"); @@ -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 diff --git a/test/chplcheck/SimpleDomainAsRange.chpl b/test/chplcheck/SimpleDomainAsRange.chpl index 3249c8cdb3c..397090ba5ed 100644 --- a/test/chplcheck/SimpleDomainAsRange.chpl +++ b/test/chplcheck/SimpleDomainAsRange.chpl @@ -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} {} diff --git a/test/chplcheck/SimpleDomainAsRange.good-fixit b/test/chplcheck/SimpleDomainAsRange.good-fixit index 7b3a1faebc4..f48662fd636 100644 --- a/test/chplcheck/SimpleDomainAsRange.good-fixit +++ b/test/chplcheck/SimpleDomainAsRange.good-fixit @@ -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} {} diff --git a/test/chplcheck/UnusedTaskIntent.chpl b/test/chplcheck/UnusedTaskIntent.chpl new file mode 100644 index 00000000000..1840f03bda0 --- /dev/null +++ b/test/chplcheck/UnusedTaskIntent.chpl @@ -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) { } +} diff --git a/test/chplcheck/UnusedTaskIntent.good b/test/chplcheck/UnusedTaskIntent.good new file mode 100644 index 00000000000..bc1dc655d47 --- /dev/null +++ b/test/chplcheck/UnusedTaskIntent.good @@ -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] diff --git a/test/chplcheck/UnusedTaskIntent.good-fixit b/test/chplcheck/UnusedTaskIntent.good-fixit new file mode 100644 index 00000000000..470388e4ce3 --- /dev/null +++ b/test/chplcheck/UnusedTaskIntent.good-fixit @@ -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) { } +} diff --git a/test/chplcheck/activerules.good b/test/chplcheck/activerules.good index 69e8dcc60c5..f2075f25265 100644 --- a/test/chplcheck/activerules.good +++ b/test/chplcheck/activerules.good @@ -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 '(_, _)'. diff --git a/test/chplcheck/allrules.good b/test/chplcheck/allrules.good index c51c4923bae..3640f817320 100644 --- a/test/chplcheck/allrules.good +++ b/test/chplcheck/allrules.good @@ -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. diff --git a/tools/chplcheck/src/rules.py b/tools/chplcheck/src/rules.py index 9f585aeb540..a9893af8a3d 100644 --- a/tools/chplcheck/src/rules.py +++ b/tools/chplcheck/src/rules.py @@ -619,6 +619,84 @@ def UnusedFormal(context: Context, root: AstNode): anchor = formals[unused].parent() yield AdvancedRuleResult(formals[unused], anchor) + @driver.advanced_rule + def UnusedTaskIntent(context: Context, root: AstNode): + """ + Warn for unused task intents in functions. + """ + if isinstance(root, Comment): + return + + intents = dict() + uses = set() + + for intent, _ in chapel.each_matching( + root, set([TaskVar, ReduceIntent]) + ): + # task private variables may have side effects, + # so we don't want to warn on them + if isinstance(intent, TaskVar): + if intent.intent() == "var": + continue + if intent.intent() == "" and ( + intent.type_expression() is not None + or intent.init_expression() is not None + ): + continue + intents[intent.unique_id()] = intent + + for use, _ in chapel.each_matching(root, Identifier): + refersto = use.to_node() + if refersto: + uses.add(refersto.unique_id()) + + for unused in intents.keys() - uses: + taskvar = intents[unused] + with_clause = taskvar.parent() + task_block = with_clause.parent() + + # only loops can be anchors for attributes + anchor = None + if isinstance(task_block, chapel.Loop): + anchor = task_block + yield AdvancedRuleResult( + taskvar, anchor, data=(with_clause, task_block) + ) + + @driver.fixit(UnusedTaskIntent) + def RemoveTaskIntent(context: Context, result: AdvancedRuleResult): + """ + Remove the unused task intent from the function. + """ + assert isinstance(result.data, tuple) + with_clause, task_block = result.data + + fixit = None + # if the with clause only has one expr, remove the entire with clause + if len(list(with_clause.exprs())) == 1: + with_loc = with_clause.location() + # header_loc is the location of the block header without the `with` + # e.g. `forall i in 1..10`, `begin`, `cobegin` + header_loc = ( + task_block.header_location() + if isinstance(task_block, Loop) + else task_block.block_header() + ) + if header_loc is not None: + start = header_loc.end() + end = with_loc.end() + else: + start = with_loc.start() + end = with_loc.end() + + fixit = Fixit.build(Edit(with_loc.path(), start, end, "")) + + else: + # for now, locations are messy enough that we can't easily cleanly + # remove the taskvar + pass + return [fixit] if fixit else [] + @driver.advanced_rule def UnusedLoopIndex(context: Context, root: AstNode): """ @@ -827,10 +905,10 @@ def unwrap_intermediate_block(node: AstNode) -> Optional[AstNode]: # var x: int; # } elif parent_depth and depth == parent_depth: - # conditionals do not support attributes + # only loops and NamedDecls can be anchors for indentation anchor = ( parent_for_indentation - if not isinstance(parent_for_indentation, Conditional) + if isinstance(parent_for_indentation, (Loop, NamedDecl)) else None ) yield AdvancedRuleResult(child, anchor=anchor)