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.

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

Allow comparison clause in if statements which contain statement-local declarations? #20645

Open
TurkeyMan opened this issue Jan 7, 2025 · 7 comments · May be fixed by #20653
Open

Allow comparison clause in if statements which contain statement-local declarations? #20645

TurkeyMan opened this issue Jan 7, 2025 · 7 comments · May be fixed by #20653

Comments

@TurkeyMan
Copy link
Contributor

TurkeyMan commented Jan 7, 2025

I find myself using statement-local declarations in if clauses a lot:

if (auto x = resolveScopeLocalValue())
{
  // ...
  // x is destroyed
}

It's natural to want some values used in the determination of the condition to only have a life that matches the if statement.

The problem is that the current language only allows implicit comparison for truth by coersion to bool. I'd like to see a second clause added to the if statement which would allow you to write the actual condition after the initialisation of the scope-local variable:

if (auto x = resolveScopeLocalValue(); [condition_expression])
{
   // ...
   // destroy x
}

The implicit condition written explicitly:

if (auto x = resolveScopeLocalValue(); x == true)
{
}

There are really common sequences that cause the current syntax to fail, ie:

if (auto x in collection; x != null && x.isWhatIWant())
{
  //...
}

Since in expressions yield a pointer which may be null in cases where the item doesn't exist, the implicit check just determines if it exists, which is not necessarily the actual condition of interest. There are all kinds of patterns like this which are incompatible with if-statement-local declaration.

It feels natural to use semicolon inside of control statements this way; we do it in all the others.
Ie, for ([declarations]; [condition]; ...), it's symmetrical with a for statement. (just without the increment part)

My workaround sometimes looks like this:

  // other code...
  {
    auto x = resolveScopeLocalValue();
    if (x == true)
    {
      //...
    }
    // destroy x
  }
  // other code...

...and obviously that's stupid, let's outgrow this.

@thewilsonator
Copy link
Contributor

C++ does something (exactly?) like this right? Or at least there was a proposal to do so. Don't have a link handy at the moment. Will update once I find it.

@TurkeyMan
Copy link
Contributor Author

TurkeyMan commented Jan 7, 2025

C++ has something like this for 10 years or so.
I'm not sure about "exactly" like this; I would propose that the semantics of the declaration and condition clauses exactly match the current semantics in the for statement, just for symmetry and predictability reasons.
C++'s statements are probably better than that; accepting more possibilities in the declaration clause, but I reckon any improvement beyond the current for capabilities (closing the gap on C++) should apply to for as well, and that's a separate matter.

@thewilsonator
Copy link
Contributor

cppreference tells me C++17 added init-statement to if.

Presumably this enhancement request also applies to switch as well (following C++). Interestingly C++ does not allow it for while, but I suppose at that point you might as well use for.

This being a grammar change requires a DIP IIRC. However it shouldn't be too hard to implement, as you note it is a fairly trivial lower from if (init; test) { ... } to { init; if (test) { ... }}.

I'll see what I can do.

@TurkeyMan
Copy link
Contributor Author

We ALREADY support if-clause declarations; the only thing we can't do is specify the condition. If you write:

if (auto x = expr) ...

What that does right now is check x == true after the initialization... so we already have this apparently, just can't specify a proper comparison.

@dkorpel
Copy link
Contributor

dkorpel commented Jan 7, 2025

I've been wanting a slightly different version of this proposal for a long time: simply allowing multiple assignConditions.

Take for example this code from dmd:

    if (e.e1.type.toBasetype().ty == Tsarray)
        if (auto ve = e.e1.isVarExp())
            if (auto vd = ve.var.isVarDeclaration())
                if (vd.storage_class & STC.ref_)
                    goal1 = CTFEGoal.LValue;

Not only is the deep nesting aesthetically unpleasing, it becomes actually problematic when you want to create an else clause for the whole thing. With multiple conditions this is easy:

if (e.e1.type.toBasetype().ty == Tsarray;
    auto ve = e.e1.isVarExp());
    auto vd = ve.var.isVarDeclaration());
    vd.storage_class & STC.ref_)
{  
    goal1 = CTFEGoal.LValue;
}
else
{
    
}

I think this would also allow your use cases, though it does require the variables to be able to cast to bool and evaluate to true. However, the ; in the if condition would be somewhat redundant with the && operator, and basile-z noted when adding declarations to switch statements:

It was not the case in the 2013 but 10 years later adding this feature is a failure to recognize the more general pattern: the VarDecl as used in this proposal could be a primary expression and should not be tied to specific statements.

  1. First this was possible as IfStatement condition.
  2. Then it was added as WhileStatement condition
  3. Now it will be added as SwitchStatement condition
  4. In the future someone will realize that
    • this is useful to deconstruct tuples
    • this is useful in AndEnExpression and OrOrExpression
    • this is useful in the CallExpression for the out parameters

etc. etc.

Making the new primary would fix that in one shot.

That idea would work for my code example:

if (e.e1.type.toBasetype().ty == Tsarray &&
    auto ve = e.e1.isVarExp()) &&
    auto vd = ve.var.isVarDeclaration()) &&
    vd.storage_class & STC.ref_)
{  
    goal1 = CTFEGoal.LValue;
}
else
{
    
}

And also your example:

if (auto x = key in collection && x.isWhatIWant())
{
  //...
}

(The null check is implicit, because cast(bool) x has to evaluate to true for the right hand side of the && to evaluate)

thewilsonator added a commit to thewilsonator/dmd that referenced this issue Jan 8, 2025
@thewilsonator
Copy link
Contributor

initial stab #20653

thewilsonator added a commit to thewilsonator/dmd that referenced this issue Jan 8, 2025
thewilsonator added a commit to thewilsonator/dmd that referenced this issue Jan 8, 2025
@thewilsonator
Copy link
Contributor

Regarding other control flow statements, I think we should make a subclass of Statement that IfStatement, SwitchStatement, ForStatement, WhileStatement, etc (?) all inherit from.

@dkorpel dkorpel linked a pull request Jan 10, 2025 that will close this issue
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants