The fix is something like "if we're going to emit this error, check to see if the thing that can't be assigned has the same name as a writable property on self, matching the class- or instance-ness". @xedin, think that's simple enough for a starter bug, or is there more nuance than that?

@belkadan Since today we are trying to diagnose based on the expressions, it might be tricky to figure out if destination type of the assignment expression is a property, but it's definitely doable, let's mark it as starter bug and if anything I'll help out to anybody who'd want to work on this.

Comment by Mayur Dhaka (JIRA)

@xedin I'd like to pick this up and start to working on it (I've been bitten by this multiple times).

I have no experience contributing to Swift though. Any help is appreciated!

mayurdhaka.swift (JIRA User) Absolutely! Let me know if you need any help, you can also open PR with the code you have and we'll discuss there.

Comment by Mayur Dhaka (JIRA)

@xedin Thank you! I have no clue which file I need to start looking at to fix this problem.

Also, can I ask you about the minutiae via some other channel since I don't want to end up polluting this thread with no valuable information.

mayurdhaka.swift (JIRA User) We can discuss in the PR or we can discuss here as well, either way is fine by me.

Comment by Mayur Dhaka (JIRA)

Alright, let me clone, the project and file a basic PR and we can continue there.

Meanwhile, could you point me in the direction I should be looking at to fix this problem? Thanks again.

You can take a look at how `FailureDiagnosis` works, it starts at `ConstraintSystem::diagnoseFailureForExpr`. In particular what happens in `visitAssignmentExpr`, it has to check if destination of the assignment is contextually a field of a enclosing nominal type using (DC field of the ConstraintSystem attached to FailureDiagnosis), and if so and it's mutable provide a fix it to add `self.`.

Comment by Mayur Dhaka (JIRA)

Thanks a bunch for your help. I've located the file and even the code that you were talking about. I didn't find `visitAssignmentExpr` anywhere in the file (CSDiag.cpp). Here are some further things I'd want to clarify:

• Where are the strings for the error messages defined? (This is where I'll be adding the fix-it message)

• How can I run the code and test out the changes I made? Breakpoints, etc?

Thank you once again!

Sorry, it's actually called `visitAssignExpr` and located at https://github.com/apple/swift/blob/master/lib/Sema/CSDiag.cpp#L7049.

Answers to the bullet points:

1). include/swift/AST/DiagnosticsSema.def declares all of the error messages used by semantic analysis.
2). I prefer to do following:

• For debugging: compile swift in debug mode (-d flag), run REPL and attach to the REPL process using Xcode debugger;

• To run tests you can use ./utils/build-script and add -t (runs unit tests test/ directory), -T (runs both unit and validation tests located in test/ and validation-test/ directories), and/or --test-paths <path>,<path> runs specific tests in the given directories/files e.g. `./build-script -d --test-paths test/Constraints/construction.swift` to build debug compiler with debug stdlib and run a set of tests contained in test/Constraints/construction.swift file.

It would be best if you add the test case from this issue to test/Constraints/construction.swift so we have it for the future.

Comment by Mayur Dhaka (JIRA)

I'm getting a little further into understanding how to navigate my way through the project. Here's a few more questions:

• In `bool FailureDiagnosis::visitAssignExpr(AssignExpr *assignExpr)` why does the method return a `bool`?

• Line 7052 has this code `diagnose(assignExpr->getLoc(), diag::cannot_assign_to_literal);`. I'm assuming the `cannot_assign_to_literal` bit is a reference to an error message but a quick Cmd+F in DiagnosticsSema.def didn't lead me to any matching error.

There's more I'd like to ask but I want to get the Swift project built first ( with the Swift.xcodeproj file at hand). To be clear, I should build with utils/build-script -dx right? Implying that I want the debug build (as you said) and have an Xcode project at hand?

mayurdhaka.swift (JIRA User)

> In `bool FailureDiagnosis::visitAssignExpr(AssignExpr *assignExpr)` why does the method return a `bool`?

`bool`, in this case, identifies if diagnostic has been produced or not. If one the visit* methods returns true the diagnostic engine is stopped.

> Line 7052 has this code `diagnose(assignExpr->getLoc(), diag::cannot_assign_to_literal);`

Don't use "diag::" prefix when searching, cannot_assign_to_literal is located on Line 216 of include/swift/AST/DiagnosticsSema.def

> To be clear, I should build with utils/build-script -dx right? Implying that I want the debug build (as you said) and have an Xcode project at hand?

-x is already going to make it a debug Xcode build, so you don't have to include -d.

Comment by Mayur Dhaka (JIRA)

@xedin thanks for the answers. I understand a bit more now where things are placed in the project (at least as they relate to the work I've to do).

Coming back to the solution you provided:

> it has to check if destination of the assignment is contextually a field of a enclosing nominal type using (DC field of the ConstraintSystem attached to FailureDiagnosis)

What does 'a field of an enclosing nominal type' mean? And where how does 'contextually' matter (and come in the picture?)

For now, I've added an error in `DiagnosticsSema.def` to the best of my ability. I've raised a PR here: #12364

> What does 'a field of an enclosing nominal type' mean? And where how does 'contextually' matter (and come in the picture?)

Since we are looking at the expression type checker it works with "bar = bar" individually, the problem here is that "bar" used as a label which, in Swift, is transformed into "let" declaration in the body of the function e.g. `func foo(bar: Int) { ... }` is actually very simplified is like this `func foo(_ arg: (Int)) { let bar = arg.$0 }` that's why when type-checker sees "bar = bar" it thinks that this is an attempt to assign a value to immutable variable "bar".

Solver itself doesn't account for enclosing scope where field "bar" is defined, but there is a way to reach it using "DC" field which represents declaration context of the expression, you can do DC->dumpContext() to see what kind of information does it have. So the idea is to get a closest nominal (class/struct) type from the context and see if "bar" is defined as a field in there, if so and type of field matches the type of "bar" from the function declaration provide a fix-it which inserts "self." before left-hand side of the "=" operator.

Comment by Mayur Dhaka (JIRA)

Thanks, that clears things up a bit for me.

> func foo(bar: Int) { ... }` is actually very simplified is like this `func foo(_ arg: (Int)) { let bar = arg.$0 }

So will the same check work for the case `func foo(bar: Int) { ... }` and `func foo (myBar bar: Int) { ... }` ? Since, as you say, Swift simplicity coverts `func foo(bar: Int) { ... }` to essentially `func foo(_ bar: (Int))` ?

My check will run irrespective of whether the variable name and the label name are the same, as in `func foo(bar: Int) { ... }`, or if the label name and the variable name are different, as in `func foo (myBar bar: Int) { ... }`

Also, I'm still not clear on how do I get the build target to run. I have my Xcode project with its massive list of targets at hand but when I build and run, there is no 'Swift 4.0(Mayur's version)' product (excuse the naive way of putting my point across) that it produces for me to run an execute to start playing around with and see which method does what (something as simple as viewing what `dumpContext()` has to offer).

The documentation doesn't make any mention of how to run Swift in the project's README.md, and neither do the docs on Swift.org. Is there something I'm missing?

> func foo (myBar bar: Int) { ... }

"myBar" in this case is an API label, the actual label used in the body is going to be "bar" still, but regardless you'd only want to do the check if the left and right hand side declarations point to the same name.

> Also, I'm still not clear on how do I get the build target to run.

There is a ALL_BUILD target which builds whole project including stdlib, and there is "swift" target which only builds/runs compiler binary. You'd want to build everything with ALL_BUILD first and then use "swift" to build/run the compiler (you can give it .swift file as an argument, could be done via "Edit Scheme" menu item).

Comment by Mayur Dhaka (JIRA)

> There is a ALL_BUILD target [...]

Yes this is what is mentioned in the README too. I found the `All_Build` scheme but I don't see any `swift` scheme.

However, `swift` is a target

(Apologies, I'm not able to inline the screenshots.)

mayurdhaka.swift (JIRA User) `swift` target is what you want, that is going to build swift executable, you can edit target scheme to add parameters to `swift` executable and run from Xcode in debug mode.

Comment by Mayur Dhaka (JIRA)

I've found the `swift` target and I've been able to provide a `.swift` file as a launch argument too. Writing the code written in this bug report's description gets me the errors when I run the `swift` scheme in Xcode too–all good on those fronts.

I'm not able to use `CS.DC>dumpContext();` inserted within the `visitAssignExpr` function to actually print any information on the console though. Any reason why that isn't happening?-

Also, I've put a breakpoint (via the GUI in Xcode) in `visitAssignExpr` but that isn't working either. I know I'm probably messing this one up since putting breakpoints this way (as one would in an iOS app) doesn't work the same way while building/running swift?

Scratch that, I've got breakpoints to work via the regular Xcode way, using the UI. It's just that the breakpoint never actually comes to `visitAssignExpr` since (I'm assuming) the code is never called? The code I'm running in my `.swift` file is, simply:

`

struct Foo {

private var bar : Int



init(bar: Int) {

    bar = bar

}

}`

mayurdhaka.swift (JIRA User) Apparently it doesn't go all the way to `FailureDiagnosis`, the problem is diagnosed by `ConstraintSystem::diagnoseAssignmentFailure` when constraints for assignment are generated you can put the breakpoint at the beginning of that method.

mayurdhaka.swift (JIRA User) What helps to find a place to start with problems like this is to do a text search for a part of swift's error message that looks unchanging, like "cannot assign to value". That will lead you to a .def file that has the error definitions in it, in this case DiagnosticsSema.def. The first identifier in the ERROR() definition there is a symbol for the error "assignment_lhs_is_immutable_variable". Then you do a project text search for that, and it leads you to ConstraintSystem::diagnoseAssignmentFailure like Pavel suggested.

Comment by Mayur Dhaka (JIRA)

@gregomni Thank you so much for your help. I've been trying something similar to navigate my way around the code and I've picked up on a few things. It didn't strike me to go about finding where the error was being called.
Again, thanks!

@xedin Yes I've found it thanks, I'm investigating further.

Comment by Mayur Dhaka (JIRA)

@xedin I've been working through this and here's where I'm stuck:

To simulate the problem I'm working with a simple example:

struct Foo {
private var bar : Int

init(bar: Int) {
bar = bar
}
}

Now, I know that the way I might want to solve this, as you mentioned previously, is to get the enclosing type (`Foo` in our case) and see if the destination (rhs `bar` in the initialiser) belongs to the enclosing type.

However, in `diagnoseAssignmentFailure` (line 4345), I'm working with three parameters: `Expr *dest`, `Type destTy`, and `SourceLoc equalLoc`. None of these seem to provide any way to reach the enclosing type so I could check for `bar` being defined there.
Additionally, I looked through `computeAssignDestType`(line 2424 in TypeCheckConstraints.cpp), the method that in turn calls `diagnoseAssignmentFailure` to see if I could reach the enclosing type from there, somehow but the parameters in that method are of the same type as `diagnoseAssignmentFailure`. How can I reach the enclosing type from either of these methods?

Also, doesn't it make more sense to write the fix within `computeAssignDestType`, not `diagnoseAssignmentFailure` since `diagnoseAssignmentFailure`'s job seems to start after the destination is already determined to be invalid?

Comment by Mayur Dhaka (JIRA)

@xedin Any update on the previous issue? I'd appreciate it, thanks!

mayurdhaka.swift (JIRA User) Sorry, I'm out this week but I'll try to take a look a bit later!

mayurdhaka.swift (JIRA User) As I was mentioning before - I think what you can do is get declaration context of the constraint system (CS->DC) and use it to acquire type of "Foo" in this case and use `lookupMember` function to lookup member with the name of "bar", if such field exists you can diagnose it.

Comment by Mayur Dhaka (JIRA)

@xedin, sorry for the intrusion, and the late reply.

I agree that the solution you propose could get me closer to solve the problem but here's where I am currently:
The error being detected in the example I'm using gets generated at line 4355 in `CSDiag.cpp` in the method called `diagnoseAssignmentFailure`.
From the call stack, I figured out that it's the `computeAssignDestType` method in the `TypeCheckConstraints.cpp` file that makes a call to `diagnoseAssignmentFailure`.

My judgment says that I should attempt to place my fix in `TypeCheckConstraints.cpp`'s `computeAssignDestType` because `diagnoseAssignmentFailure`, as the name suggests implies that there IS an error. (We want an error with a FIXIT). That means my solution should be fit into `computeAssignDestType` but I can't access `CS->DC` from that function, and simply accessing `DC->...` doesn't let me see a `lookupMember` function.

I could try to create a new method in `CSDiag.cpp` and call it, the same way `diagnoseAssignmentFailure` is called but that seems like too huge a change to the codebase to me from where I stand so I wanted to swing that solution by you first so I know what you think of that.

Also, an aside: the documentation for `computeAssignDestType` reads:
> Compute the rvalue type of the given expression, which is the destination of an assignment statement.

This doesn't add up for me. If I have a line of code `foo = bar`, `foo` is the destination, and `bar` is the rvalue, right? The documentation says 'value type of the given expression, which is the destination of an assignment'?

I can't access `CS->DC` from that function, and simply accessing `DC->...` doesn't let me see a `lookupMember` function.

`lookupMember` is a method on `ConstraintSystem` itself, it accepts two arguments Type of base and DeclName of the method. `DC` is only required to figure what "base" Type is in this case, It doesn't matter where to you it really, but I'd prefer all diagnostic logic be in diagnose* methods.

Compute the rvalue type of the given expression, which is the destination of an assignment statement.

What this is trying to say that `computeAssignDestType` trying to compute the type which is going to be wrapped into @lvalue by top level code. So e.g. if you have `var foo = 42;` translated to types it's `@lvalue T = LiteralInt`, so `computeAssignDestType` is responsible for trying to figure out the concrete type of `T` is or use type variable with specific constraints.

Resetting assignee on all Starter Bugs last modified in 2018 or earlier.

This was addressed in 8d70f98 by Greg. There's a much much better diagnostic here

error: repl.swift:5:9: error: cannot assign to value: 'bar' is a 'let' constant
        bar = bar
        ^~~

repl.swift:5:9: note: add explicit 'self.' to refer to mutable property of 'Foo'
        bar = bar
        ^
        self.

Closing this out.