-
Notifications
You must be signed in to change notification settings - Fork 4k
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.
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
Proposal: guard statement #11562
Comments
I'm thinking the keyword 'assert' would read better here than 'guard'. I'm not sure what guard is guarding.. it seems like it is named after the jargon-y term named after the common pattern of writing code. And I transpose the u and the a when typing guard all the time. Where as 'assert' is nice because it reads like I'm making a claim about the expression being true and implies that there is nothing special to be done when it is so. |
@mattwar An assertion is making a claim about the expression, but the guard doesn't make such a claim. It tests whether the condition is true or not, expecting that it could be either, and provides code to handle the "not true" case and "get out of there". |
Why not just: guard (expr)
statement ? The 'else' reads super weird to me. With the above i can then write: guard (n != null)
throw new ArgumentNullException(n) Or guard (int.TryParse(out var v)) {
return;
}
// use v |
@CyrusNajmabadi That sounds like Perl's |
@gafter It seems to work just like a Debug.Assert expression, except it not debug and I get to say what happens when it fails. That's a strong association that every programmer already has. Guard just doesn't tell me what it does. It tells me I have to do a web search to find out. |
Also, i thnk i like 'unless' as well. it reads nicely. "unless n is not null, throw this thing. unless this int parses, return from what i'm going." 'guard' doesn't immediately translate well in my head. |
Plenty of discussion over on #8181 which however does not discuss the scope rules. Also related is #6400 and the |
Wouldn't it be simpler and more consistent if To me this makes the most sense: |
@svick The same issues occur with pattern-matching, and for the same reasons. We're pretty comfortable with the tentative scope rules we have today, but this is one particular use case where they rub us the wrong way. |
I want to offer a counter-proposal which I think is simpler and satisfies most of the same scenarios: _Proposal: Lifting of Example: TryGet dictionary.TryGet("key", out var value);
// easier than the existing code:
Tuple<Func<string,bool>, string, string> value;
dictionary.TryGet("key", out value); Example: pattern matching if (!(o is out string s)) return;
Console.WriteLine(s); Example: guards if (!int.TryParse(input, out int i)) throw new ArgumentException(nameof(input));
Console.WriteLine(i); Unexample: limited scope out varaibles if (int.TryParse(input, out int i)) { Console.WriteLine(i); }
// Under this proposal, there's no way to limit the scope of "out arguments" to just the 'then' block
if (o is string s) { Console.WriteLine(s); }
// But there is still a way to limit the scope of pattern variables to just the 'then' block Example: loop for (out int i=0; i<10; i++)
{
if (stop_early) break;
}
Console.WriteLine($"We got to {i}"); Example: foreach loop foreach (out var x in GetEnumeration(out var dummy)) { ... }
// Note that "x" isn't definitely assigned after the foreach (in case the enumeration was empty).
// I just put this example to show that the rules are uniform: the "out" modifier
// always pushes out the variable's scope, regardless if it's the loop variable
// or an out variable Example: normal variable declaration void f() {
out int x = 15;
}
// Although technically meaningful, it has the same effect as just "int x = 15" here,
// and I think it should be disallowed as confusing. The idea is that in many cases (e.g. patterns, "is" testing, "then" clauses) then there's no need to expand to the enclosing scope. But in many cases (e.g. guards) there is. This proposal gives the user control of whether the variable's scope expands out, in an at-a-glance understandable syntax. As the "unexample" above shows, there's one corner where this proposal doesn't satisfy: out arguments with limited scope. I think this corner is small enough (and the penalties of omitting this corner small enough) that it's worth the sacrifice to get a simple feature. |
This is useful for out variable declaration, but I'd prefer let (string key, int value) = expression else continue;
guard (int.TryParse(str, out var parsedValue)) else continue; In both of these examples, the end of embedded-statement must be unreachable, so in my opinion I'll also note that |
The one thing I like about guard is that it serves as a helper for validation logic to ensure that the method does bail out: private void Foo(string s) {
if (s == null) {
Log.Warn("s is null!");
// oops, forgot to return or throw
}
int l = s.Length; // boom
}
private void Bar(string s) {
guard (s == null) else {
Log.Warn("s is null");
// compiler error, must use throw, return, break or continue to exit current block
}
int l = s.Length;
} This proposed behavior that pattern and As for the |
It seems all we need is to invert the scope of the if not (int.TryParse(s, out var parsedValue))
{
// handle or report the problem
return;
}
// use parsedValue here In this way, you don't need to introduce a completely new statement, but only an alternative of |
@qrli You're still introducing a new statement and a new keyword. An "inverted if" also doesn't imply that it would force exiting the current block. |
@qrli I have request But somehow I think it could possible if we just use Normally we must use I'm not native english speaker so if this sound weird I would support |
Doesn't this encourage e.g. pushing argument validation down the call chain? I would personally like to see method contracts #119 implemented first. The concepts are different of course, I understand that. Alternatively, how about D-style contracts, but changed so that variables declared in the |
@HaloFour |
@gafter Regarding "that doesn't work because More formally: Currently the patterns proposal contains the following wording:
I suggest that the scope of the variable is broadened to be as if the variable was declared just before the
So if you write the following, the semantics are already as desired: if (maybeX is Some x)
{
// x is in scope and definitely assigned
}
else
{
// x is in scope, but not definitely assigned (unusable)
} The way the specification is currently written precludes the possibility of useful code such as: void Handle(Option<T> maybeX)
{
if (!(maybeX is Some x)) return;
// x is now in scope and definitely assigned
} Or: if (!(dictionaryFromTheFuture.TryGetValue(key) is Some x))
{
x = defaultValue;
}
// x is in scope and definitely assigned And this code from the original post is now valid: Tuple<string, int> expression = ...;
if (!(expression is @(string key, int value))
{
continue;
}
// use key, value here - they are in scope and definitely assigned These examples also suggest that an |
@Porges The reasons for the current scoping rules are, among other things, to help you with code such as a series of if-then-else, so you don't have to invent a new name for each branch. |
@gafter hmm, I would have thought that that would be a rarer situation than things like the above. Thanks for the reasoning, though. Perhaps you should be permitted to shadow a non-definitely-assigned variable? 😀 |
We need to know the scoping of the variables (so we know what is being assigned) before figuring out whether they are definitely assigned or not, so that doesn't work. |
Maybe we could use void Guard(string s)
{
return if(!int.TryParse(s, out var value))
{
// handle or report the problem
// auto return after this block
}
// use value
}
int GuardError(string s)
{
return if(!int.TryParse(s, out var value))
{
// handle or report the problem
// ERROR it must return something
}
// use value
}
int Guard(string s)
{
return if(!int.TryParse(s, out var value))
{
// handle or report the problem
return 0; // need to return value
}
// use value
}
int GuardReturn(string s)
{
// Maybe this syntax
return (0) if(!int.TryParse(s, out var value))
{
// handle or report the problem
}
// use value
}
int GuardReturn(string s)
{
// Or this syntax
return if(!int.TryParse(s, out var value))
{
// handle or report the problem
0; // Auto return last line
}
/* Above code would be transpiled
if(!int.TryParse(s, out var value))
{
// handle or report the problem
return 0;
}
*/
// use value
// So we could
return if(!int.TryParse(s, out var value))
{
int.Parse(0); // Auto return last line of block
}
} |
Just to 🚲🏠 how about when (foo == null) {
return; // throw, break or continue
} |
That doesn't seem different enough from |
@bondsbw Fair enough, I just want to avoid the |
Everyone seems like the idea. But naming is hard.. |
It is not only about naming. 1st: The 2nd: "Invert if" is a routinely used refactor. With I'd like keep it as simple if (s == null) else {
return;
} |
Why is if( Int.TryParse(s, out x) && (x > 0) Only case is in
|
The With if (int.TryParse(s, out var x)) {
// x in scope here
}
// x no longer in scope here As if (!int.TryParse(s, out var x)) {
Log.Warn("The string is not a number!");
return;
}
// x no longer in scope here So guard changes the scoping rules to allow the variable to be used in the enclosing scope: guard (!int.TryParse(s, out var x)) else {
Log.Warn("The string is not a number!");
return;
}
// x is in scope here And since |
@HaloFour Didn't you forget to write |
@stepanbenes That I did, I will fix the examples. |
@HaloFour Thank you for explaining the difference but I feel that my point still stands. It would easier to just define the variable to use in the out variable position. In the existing scope. int value = 0;
if( !Int.TryParse( text, out value ) )
{
// log stuff
return ;
}
// do stuff with value The example of guards proposed assume the usage ( of out variables ) will be in a conditional expression like
Also having |
Inclusive Scope if (int.TryParse(input, out int i)) { Console.WriteLine(i); }
// Under this proposal, there's no way to limit the scope of "out arguments" to just the 'then' block Exclusive scope if (int.TryParse(input, put int i)) { Console.WriteLine(i); }
// Under this proposal, there's no way to limit the scope of "out arguments" to just the 'then' block |
For
Per the
Considering that's mostly the point of |
@HaloFour Explicitly Local if( Int.TryParse( text, [local] out var value ) )
{
// value is defined and assigned within here.
}
else
{
// value is not define here.
} Guard if( !Int.TryParse( text, [guard] out var value ) )
{
// value is not defined here.
}
else
{
// value is defined and assigned |
I would go against attribute syntax. It too dirty and complicate For local scope I think maybe we could add more syntax, maybe if( Int.TryParse( text, out let value ) )
// value is defined and assigned within here.
else // value is not define here.
// value is not define here ? maybe For guard I still stand with |
@Thaina It's not about return the value, but into scope(s) should the Also I don't think it would go against attribute syntax, as I see the attribute is attached to the out var parameter argument. |
@AdamSpeight2008 Yes I know it not about And about attribute, I would not like it if attribute can cause syntax error IMHO. It should be syntax keyword that could go that far |
@Thaina if( Int.TryParse( text, local out var value ) )
if( !Int.TryParse( text, guard out var value ) ) |
or at least Shorter and cleaner guard is another problem, it cannot do that syntax. it not parameter specific. it is scope specific |
@gafter @HaloFour let bool(true) = int.TryParse(s, out var parsedValue) else return;
// scope of out var is treated the same as the returned variable, so they are all accessible here But the pattern part looks a bit weird. I guess the pattern syntax could extended to support function call, though I do not have a concrete idea yet. The ideal imagination for me is: let int.TryParse(s, out var parsedValue) else return; |
@qrli Your idea is neat But it not satisfied the proposal to do some work and log something before return or throw |
@Thaina Perhaps
|
I don't like this idea that variables can start escaping their scope (or what it looks like their scope should be) if certain keywords were used... I think it makes the code more difficult to read. I'm not sure it deserves a new language keyword / construct just to avoid having to write
Now that the language has Tuples, why don't we instead argue that methods with out parameters now appear to return Tuples? (FYI this is what FSharp does.) After all, the only reason out parameters exist is because the language didn't have tuples from the beginning. For example, calls to Int32.TryParse would become:
So much cleaner and easier to understand, especially with pattern matching (if I can take a guess at the syntax:
|
Per #12597 the conversation is probably mostly moot. Now the scope will sometimes leak out into their enclosing scope based on which constructs you use: if (!int.TryParse(s, out int parsedValue))
return;
// use parsedValue here I personally don't agree with it. It's inconsistent with existing C# behavior, it's inconsistent with itself and it's inconsistent with the rest of the C-family of languages. I'd rather the team did nothing at all and if the user intended to "leak" the scope that they would declare the variable separately, just as you demonstrated. |
@HaloFour that's a shame... By the way,
did you mean "I personally don't agree with it"? |
That I did. |
Closing, as this is no longer useful with the new scoping rules. |
When reviewing the scoping rules for
out
variables and pattern variables, the LDM discovered that there is sometimes an unfortunate interaction with a pattern of coding that we call the guard pattern. The guard pattern is a coding pattern where you test for the exceptional conditions early, handle them, and then bypass the rest of a method by returning, the rest of a loop by continuing, etc. The idea is to not have to deeply nest the "normal" control path. You'd write code like this:To take advantage of the
out var
feature, and avoid having to write the type of theout
variable, you'd like to writebut that doesn't work because
parsedValue
isn't in scope in the enclosing statement. Similar issues arise when using pattern-matching:(The
@
here assumes that tuple patterns are preceded by an@
character)but that doesn't work because
key
andvalue
aren't in scope after theif
statement.Swift addresses this category of issue by introducing the
guard
statement. Translating that into the concepts we've been proposing for C# with tuples and pattern-matching, the equivalent construct would be a new kind of statement:A
guard
statement is like an inverted if statement, or an if statement without a "then" block. It has the following semantics:false
.true
.This would allow you to handle the previous examples as follows:
/cc @dotnet/ldm
The text was updated successfully, but these errors were encountered: