Conversation
| var output = EvalReturningOutputString(source); | ||
|
|
||
| Assert.Equal("nil", output); | ||
| } |
There was a problem hiding this comment.
Other use cases are more tricky. For example, should we or should we not allow
nilassignment by default for reference types? We'll try to decide on this before merging this PR.
In other words, this. ☝️ We need to figure out whether this is what we want or not. Having it like this is the easiest way out, but it should actually be quite simple to forbid all nil assignments also if we like. We could actually give it a try and see how many tests it breaks. 💡
It's probably much easier to forbid nil assignment at this early stage in the Perlang development than trying to get rid of it after-the-fact, which experience from other languages has proven to be quite hard (not to say impossible). (Well, C# has made it possible to default to non-nullability by default which is quite an achievement.)
There was a problem hiding this comment.
The following is now in place:
- Reassignments of variables to
nilemits a warning. - Using
nilas a variable initializer likewise emits a warning - Bug fix: Using
nilfor local variable inference (var s = nil) is now properly detected as an invalid condition, causing a compilation error. There is simply no way for the compiler to determine what type the user was intending to use in that case, so it's better to just fail fast. - Passing
nilas a function parameter also emits a warning.
The PR got a bit more complex because of this. I'll try to extract the infrastructure for emitting warnings into a separate PR in a day or two, since it's an isolated piece of art that doesn't necessarily need to be glued together with the rest like this.
One big caveat at the moment is also the following semantic detail:
- In REPL mode, warnings are treated as warnings and just produce some noise; the code will still run as if the warning wasn't there.
- In script mode, warnings are treated as errors (imagine
-Werrorbeing enabled withgcc) and there is no way to turn it off. The latter might be a bit unpleasant and we could try to fix a-Wno-error-flag or something when we have the infra for warning extracted to an isolated PR.
There was a problem hiding this comment.
One big caveat at the moment is also the following semantic detail:
- In REPL mode, warnings are treated as warnings and just produce some noise; the code will still run as if the warning wasn't there.
- In script mode, warnings are treated as errors (imagine
-Werrorbeing enabled withgcc) and there is no way to turn it off. The latter might be a bit unpleasant and we could try to fix a-Wno-error-flag or something when we have the infra for warning extracted to an isolated PR.
This was now fixed. I added a -Wno-error=nil-usage flag that you can use to demote usage of nil to just a warning instead of an error.
The REPL still always treats it as a warning which makes sense in my world view of how a REPL should "feel" to the user.
One caveat still: the EvalHelper methods will consider all warnings warnings by default, not errors. We could change this (and perhaps we should) but it does berak about 5 unit tests. Some of them could easily be fixed but not others. We would probably have to figure out a way to disable/enable warnings for individual tests to accomplish that (perhaps by moving them to be Program integration tests and passing in the proper -Wno-error flag). But, that's a different story for another day.
I'll try to extract the infrastructure for emitting warnings into a separate PR in a day or two, since it's an isolated piece of art that doesn't necessarily need to be glued together with the rest like this.
This part I'll try to deal with before we get this merged though.
perlun
force-pushed
the
fix/ensure-variables-are-not-reassigned-with-incoercible-values
branch
6 times, most recently
from
fff1220 to
eb43349
Compare
perlun
force-pushed
the
fix/ensure-variables-are-not-reassigned-with-incoercible-values
branch
2 times, most recently
from
54c29ad to
4e19384
Compare
This is an extract of most of the infrastructure code from #188, to make that PR clearer and more focused on the actual semantic changes.
perlun
force-pushed
the
fix/ensure-variables-are-not-reassigned-with-incoercible-values
branch
2 times, most recently
from
5e81fd1 to
ed3d19b
Compare
| var printOption = new Option<string>("-p", "Parse a single-line script and output a human-readable version of the AST") { ArgumentHelpName = "script" }; | ||
| var noWarnAsErrorOption = new Option<string>("-Wno-error", "Treats specified warning as a warning instead of an error.") { ArgumentHelpName = "error" }; | ||
|
|
||
| var disabledWarningsAsErrorsList = new List<WarningType>(); |
There was a problem hiding this comment.
(Silly List suffix to avoid name clash with non-static field. It's a shame the C# compiler uses the same scope for these, since a static method cannot access the fields anyway. Let's try to do better in Perlang when we implement support for static methods. 😎)
|
|
||
| if (expr.Value.TypeReference.IsNullObject) | ||
| { | ||
| // TODO: Use expr.Value.Token here instead of expr.name, #189 |
| } | ||
| else if (stmt.Initializer.TypeReference.IsNullObject) | ||
| { | ||
| // TODO: Use stmt.Initializer.Token here instead of stmt.name, #189 |
perlun
force-pushed
the
fix/ensure-variables-are-not-reassigned-with-incoercible-values
branch
2 times, most recently
from
69c6e30 to
159dfa2
Compare
A typical example of an incoercible assignment is this:
var i = 1;
i = "foo";
...in other words, an integer value being reassigned a `string` value.
Other use cases are more tricky. For example, should we or should we not
allow `nil` assignment by default for reference types? We'll try to
decide on this before merging this PR.
perlun
force-pushed
the
fix/ensure-variables-are-not-reassigned-with-incoercible-values
branch
from
159dfa2 to
588a089
Compare
perlun
deleted the
fix/ensure-variables-are-not-reassigned-with-incoercible-values
branch
1 participant
A typical example of an incoercible assignment is this:
...in other words, an integer value being reassigned a
stringvalue.Other use cases are more tricky. For example, should we or should we not allowWe settled on emitting a warning onnilassignment by default for reference types? We'll try to decide on this before merging this PR.nilassignment, which is considered an error by default but can be demoted to a warning if necessary.