Chapel: Use before def errors should also note where the used variable is defined

[lydia-duncan]

Summary of Problem

As seen in #9985, if an outer scoped variable shares a name with a more local scoped variable defined after the use, it can be confusing to tell which symbol is meant in the error message. If the more local scoped variable is defined much later in the scope than its use-before-def, it would be hard to track it down to rename it or otherwise avoid the error. It would be nice for this error message to also include the location where the matched variable is defined.

Steps to Reproduce

Source Code:

proc main() {
  var a = 12;
  {
    var b = a; // This actually refers to the variable on the following line, but the user thinks they are referring to the one above
    var a = 1;
  }
}

Compile command:
chpl foo.chpl

Associated Future Test(s):
test/visibility/scoping-shadows-error-message.chpl #10035

Configuration Information

• Output of chpl --version: 1.18.0 (pre-release

[bradcray]

I've got a fix for this cooking in PR #10051.

[bradcray]

Before I update a lot of .good files, let's agree on a wording for this error message. Lydia's future proposed:

test.chpl:11: error: 'a' used before defined (first used here, defined at line 12)

I felt like the "first used here" added some confusion without improving clarity much, so I reworded to:

test.chpl:11: error: 'a' used before defined (defined here: test.chpl:12)

which I like better, though I wonder whether two "defined"s in a row is confusing, so am looking for further thoughts and feedback. The one other thought I have tonight is:

test.chpl:11: error: 'a' is used before it is defined at test.chpl:12

[vasslitvinov]

We could follow a pattern used in several other error messages and generate this:

test.chpl:11: error: 'a' is used before defined
test.chpl:12: note: it is defined here

Short of that, +1 to 'a' is used before it is defined at test.chpl:12

[mppf]

I think I prefer the 2-line version but I don't have a strong preference. I agree about removing "first used here" but don't have a strong preference between the later 1-line versions.

[gbtitus]

Perhaps:

test.chpl:11: error: 'a' used here, before defined at line 12

[lydia-duncan]

I like Vass's version, but we could also do:
test.chpl:11: error: 'a' used before defined (definition at: test.chpl:12)

[bradcray]

@gbtitus:

Perhaps:

test.chpl:11: error: 'a' used here, before defined at line 12

Though it's a ways off yet, I feel it's important to name the filename for both the definition and the use to support the day when we have include statements or an equivalent.

[bradcray]

Belatedly, tagging @bryantlam on this since it stemmed from his issue #9985.

[gbtitus]

@bradcray - oops, yes, totally agree. Mine didn't have the 2nd filename because I just cut+pasted+edited the first msg I saw while scanning the issue.

[BryantLam]

I don't have a preference. Probably one of these two:

test.chpl:11: error: 'a' used before defined
test.chpl:12: note: defined at test.chpl:12

or

test.chpl:11: error: 'a' used before defined at test.chpl:12

I like whichever could be used more generically by other compiler error messages.

[bradcray]

Seems like there's a strong preference for Vass's two-line proposal (which I agree with). I'm now wrestling with wording. Probably too much, but I don't want to have to update 40 .goods twice after a code review suggests a better wording:

Line 1 options:
a) test.chpl:11: error: 'a' used before defined // terse
b) test.chpl:11: error: 'a' is used before defined // Vass
c) test.chpl:11: error: 'a' is used before it was defined // English (past)
d) test.chpl:11: error: 'a' is used before it is defined // English (present)
e) test.chpl:11: error: 'a' is used here before it is defined // Verbose (or could use was
f) other?

Line 2 options:
a) test.chpl:12: note: defined here // terse
b) test.chpl:12: note: it is defined here // Vass
c) test.chpl:12: note: (defined here) // parenthetical
d) other?

[vasslitvinov]

1e + 2a

[lydia-duncan]

I favor 1a and 2a

[bradcray]

That's 2-to-1 in favor of 1a and 2a if anybody else wants to weigh in before I proceed.