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

Confusing error message when method only includes variable declarations #2828

Open
patternspandemic opened this issue Jul 18, 2018 · 3 comments
Open

Confusing error message when method only includes variable declarations #2828

patternspandemic opened this issue Jul 18, 2018 · 3 comments
Assignees
@adri326
Labels
help wanted Extra attention is needed

Comments

@patternspandemic
Copy link
Contributor

ponyc version:

0.24.0 [release]
compiled with: llvm 3.9.1 -- gcc (GCC) 7.3.0
Defaults: pic=false ssl=openssl_0.9.0

source:

actor Main
  new create(env:Env) =>
    confusing_error_msg()
    
  fun confusing_error_msg(): String =>
    var s = "A string."
    
    // Forget to return a string
    //s

compiler output:

0.24.0 [release]
compiled with: llvm 3.9.1 -- cc (Ubuntu 5.4.0-6ubuntu1~16.04.10) 5.4.0 20160609
Defaults: pic=false ssl=openssl_0.9.0
Error:
main.pony:26:9: the left side is undefined but its value is used
    var s = "A string."
        ^
Error:
main.pony:26:11: left side must be something that can be assigned to
    var s = "A string."
          ^

Furthermore, changing the return type of confusing_error_msg to None results in a successful build, without any error output mentioning unused variables. I remember receiving errors regarding unused declarations previously, and am unsure if they are still a compilation error.
@aturley
Copy link
Member

aturley commented Oct 4, 2018

I think this is more generally a problem with the error message. At the very least maybe we could have a hint that says something about "destructive read" to give people a place to look? I don't necessarily think we need a specific error message for this situation (where the code indicates that the result of an initial destructive read should be returned by the function). I think the second error is also confusing (or even inaccurate) and should probably not appear.

@jemc jemc self-assigned this Nov 6, 2018
@adri326
Copy link
Member

adri326 commented Jan 16, 2020

I'd like to tackle this issue.

@SeanTAllen
Copy link
Member

@adri326 have at it!

@SeanTAllen SeanTAllen assigned adri326 and unassigned jemc Jan 16, 2020
@adri326 adri326 mentioned this issue Jan 17, 2020
Merged
SeanTAllen pushed a commit that referenced this issue Feb 1, 2020
@adri326
Improved error for undefined but used left side of declarations (#3451)
b6aa53f 
This addresses #2828

Declaring a variable at the end of a function which expects a return value or having generally a variable declaration where the compiler expects to use the return value caused in most cases an error stating that the variable is undefined but its value is used:

```
actor Main
  new create(env:Env) =>
    confusing_error_msg()
    
  fun confusing_error_msg(): String =>
    var s = "A string."
```

This bit of code triggers the following error on 0.33.1:

```
Error:
main.pony:26:9: the left side is undefined but its value is used
    var s = "A string."
        ^
Error:
main.pony:26:11: left side must be something that can be assigned to
    var s = "A string."
          ^
```

The presence of the second error was also considered to be erroneous.

This patch discriminates in the `is_lvalue` function from `pass/refer.c` cases where the left-hand side of the assignment is not an `lvalue` and cases where it triggers an error. This way, the compiler can tell between the left side not being something that can be assigned to or it having triggered an error.

Additionally, an *Info* message is triggered when the left side's old value is needed in a declaration.

This is what the error message now looks like:

```
Error:
main.pony:6:9: the left side is undefined but its value is used
    var s = "A string."
        ^
    Info:
    main.pony:6:5: the previous value of 's' is used because you are trying to use the resulting value of this var declaration
        var s = "A string."
        ^
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adri326 adri326

Labels
help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@SeanTAllen @aturley @jemc @patternspandemic @adri326