Add support for multiple `else if` and `if constexpr` #366

filipsajdak · 2023-04-13T00:39:49Z

Add support for else if branches and constexpr ifs

Added support for else if branches.
Added support for constexpr ifs
Added support for definite initialization checks
Added support for debug symbols (if branch, if else branch, else branch)
Add support for constructing variables in conditions
Add support for nested ifs/else-ifs

The current implementation of cppfront (f83ca9) supports only if with one condition:

if x > 1 {
  // true branch
} else {
  // false branch
}

This change makes it possible to construct ifs with multiple else if checks. That makes the following code:

main: (args) = {
    p : *int;

    a := 1;
    b := 2;
    c := 3;
    d := 4;

    if args.cout == 3 {
        p = a&;
    } else if p = b& {
        if args.cout == 2 {
            p = c&;
        } else {
            if b > 0 {
                p = a&;
            } 
            else {
                p = d&;
            }
        }
    } else {
        p = c&;
    }

    std::cout << p* << std::endl;
}

Generates:

auto main(int const argc_, char const* const* const argv_) -> int{
    auto args = cpp2::make_args(argc_, argv_); 
#line 2 "/Users/filipsajdak/dev/execspec/external/tests/pure2-nested-ifs.cpp2"
    cpp2::deferred_init<int*> p; 

    auto a {1}; 
    auto b {2}; 
    auto c {3}; 
    auto d {4}; 

    if (args.cout==3) {
        p.construct(&a);
    } else if (p.construct(&b)) {
        if (args.cout==2) {
            p.construct(&c);
        }else {
            if (cpp2::cmp_greater(std::move(b),0)) {
                p.construct(&a);
            }
            else {
                p.construct(&d);
            }
        }
    }else {
        p.construct(&c);
    }

    std::cout << *cpp2::assert_not_null(std::move(p.value())) << std::endl;
}

The change also brings support for debug symbols:

  0 | function main
  1 |   scope
  2 |     var p *** UNINITIALIZED
  2 |     /var 
  2 |     var a
  2 |     /var 
  2 |     var b
  2 |     /var 
  2 |     var c
  2 |     /var 
  2 |     var d
  2 |     /var 
  2 |     selection
  3 |       if branch
  4 |         *** (10,9) DEFINITE INITIALIZATION OF p
  4 |         *** use of a
  4 |         /if branch
  3 |       *** (11,15) DEFINITE INITIALIZATION OF p
  3 |       *** use of b
  3 |       if else branch
  4 |         selection
  5 |           if branch
  6 |             *** (13,13) DEFINITE INITIALIZATION OF p
  6 |             *** (13,17) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of c
  6 |             /if branch
  5 |           else branch
  6 |             selection
  7 |               *** (15,16) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of b
  7 |               if branch
  8 |                 *** (16,17) DEFINITE INITIALIZATION OF p
  8 |                 *** (16,21) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of a
  8 |                 /if branch
  7 |               else branch
  8 |                 *** (19,17) DEFINITE INITIALIZATION OF p
  8 |                 *** (19,21) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of d
  8 |                 /else branch
  7 |               /selection
  6 |             /else branch
  5 |           /selection
  4 |         /if else branch
  3 |       else branch
  4 |         *** (23,9) DEFINITE INITIALIZATION OF p
  4 |         *** (23,13) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of c
  4 |         /else branch
  3 |       /selection
  2 |     *** (26,18) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of p
  2 |     /scope
  1 |   /function

All regression tests pass.

JohelEGP · 2023-04-13T00:49:54Z

Add support for constructing variables in conditions

Looking at https://godbolt.org/z/vT8rz8brb, which fails with

main.cpp2:3:7: error: value of type 'void' is not contextually convertible to 'bool'
  if (b.construct(true)) {
      ^~~~~~~~~~~~~~~~~
1 error generated.

Wouldn't making that return an lvalue reference to the constructed object, simulating the conventional semantics of the assignment operator, have sufficed?

filipsajdak · 2023-04-13T00:58:08Z

Thanks for the feedback! I did not check if that makes sense. This is how it works before my change - I just make it work in multiple else if conditions.

filipsajdak · 2023-04-13T01:01:24Z

You are right - returning the lvalue reference will solve that issue.

filipsajdak · 2023-04-13T09:52:50Z

@JohelEGP I have thought about this failed case. I asked myself what would @hsutter say. I think he might ask if is it a bug or is it a feature.

Do we want to support initialization in the ifs condition?

filipsajdak · 2023-04-13T10:02:36Z

Currently, it will save us from the mistakes like writing i = 42 instead of i == 42.

JohelEGP · 2023-04-13T12:37:40Z

I asked myself what would @hsutter say. I think he might ask if is it a bug or is it a feature.

I do wonder. The meaning of the first Cpp2 operator='s call of a local variable changes depending on whether it was uninitialized or not. And I think it's the same for out parameters.

JohelEGP · 2023-04-14T01:16:27Z

Currently, it will save us from the mistakes like writing i = 42 instead of i == 42.

That can happen if it's not a definite first assignment.

Opened #368 for the suggestion at #366 (comment).

hsutter

Thanks! Definite initialization currently requires that an uninitialized variable be initialized on both branches or neither branch... did you test that this generalizes correctly to requiring that an uninitialized variable be initialized on all branches or no branch?

source/sema.h

filipsajdak · 2023-04-19T23:20:33Z

Yes. I have test that. That was the easy part. The thing I was strangling was the case where the initialization happened in the condition of some else-if (previously there was only one condition and it was handled as an exception).

I might add some tests to this PR as a check.

filipsajdak · 2023-04-19T23:21:51Z

I am not sure about not "no branch case" - I will check it.

filipsajdak · 2023-04-20T20:39:15Z

I have fixed nested ifs/else-ifs. The new version of the code was pushed.

Now for the following code:

main: (args) = {
    p : *int;

    a := 1;
    b := 2;
    c := 3;
    d := 4;

    if args.cout == 3 {
        p = a&;
    } else if p = b& {
        if args.cout == 2 {
            p = c&;
        } else {
            if b > 0 {
                p = a&;
            } 
            else {
                p = d&;
            }
        }
    } else {
        p = c&;
    }

    std::cout << p* << std::endl;
}

It generates:

//=== Cpp2 type declarations ====================================================


#include "cpp2util.h"



//=== Cpp2 type definitions and function declarations ===========================

#line 1 "/Users/filipsajdak/dev/execspec/external/tests/pure2-nested-ifs.cpp2"
auto main(int const argc_, char const* const* const argv_) -> int;
    

//=== Cpp2 function definitions =================================================

#line 1 "/Users/filipsajdak/dev/execspec/external/tests/pure2-nested-ifs.cpp2"
auto main(int const argc_, char const* const* const argv_) -> int{
    auto args = cpp2::make_args(argc_, argv_); 
#line 2 "/Users/filipsajdak/dev/execspec/external/tests/pure2-nested-ifs.cpp2"
    cpp2::deferred_init<int*> p; 

    auto a {1}; 
    auto b {2}; 
    auto c {3}; 
    auto d {4}; 

    if (args.cout==3) {
        p.construct(&a);
    } else if (p.construct(&b)) {
        if (args.cout==2) {
            p.construct(&c);
        }else {
            if (cpp2::cmp_greater(std::move(b),0)) {
                p.construct(&a);
            }
            else {
                p.construct(&d);
            }
        }
    }else {
        p.construct(&c);
    }

    std::cout << *cpp2::assert_not_null(std::move(p.value())) << std::endl;
}

And creates the following symbols:

  0 | function main
  1 |   scope
  2 |     var p *** UNINITIALIZED
  2 |     /var 
  2 |     var a
  2 |     /var 
  2 |     var b
  2 |     /var 
  2 |     var c
  2 |     /var 
  2 |     var d
  2 |     /var 
  2 |     selection
  3 |       if branch
  4 |         *** (10,9) DEFINITE INITIALIZATION OF p
  4 |         *** use of a
  4 |         /if branch
  3 |       *** (11,15) DEFINITE INITIALIZATION OF p
  3 |       *** use of b
  3 |       if else branch
  4 |         selection
  5 |           if branch
  6 |             *** (13,13) DEFINITE INITIALIZATION OF p
  6 |             *** (13,17) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of c
  6 |             /if branch
  5 |           else branch
  6 |             selection
  7 |               *** (15,16) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of b
  7 |               if branch
  8 |                 *** (16,17) DEFINITE INITIALIZATION OF p
  8 |                 *** (16,21) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of a
  8 |                 /if branch
  7 |               else branch
  8 |                 *** (19,17) DEFINITE INITIALIZATION OF p
  8 |                 *** (19,21) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of d
  8 |                 /else branch
  7 |               /selection
  6 |             /else branch
  5 |           /selection
  4 |         /if else branch
  3 |       else branch
  4 |         *** (23,9) DEFINITE INITIALIZATION OF p
  4 |         *** (23,13) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of c
  4 |         /else branch
  3 |       /selection
  2 |     *** (26,18) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of p
  2 |     /scope
  1 |   /function

filipsajdak · 2023-04-20T20:44:29Z

If one of the branches is not initializing the variable, it stops with the error on the if statements that do not provide initialization in all branches. E.g.:

main: (args) = {
    p : *int;

    a := 1;
    b := 2;
    c := 3;
    d := 4;

    if args.cout == 3 {
        p = a&;
    } else if p = b& {
        if args.cout == 2 {
            p = c&;
        } else {
            if b > 0 {
                p = a&;
            } 
            else {
                // p = d&;
            }
        }
    } else {
        p = c&;
    }

    std::cout << p* << std::endl;
}

Gives the following error:

/Users/filipsajdak/dev/execspec/external/tests/pure2-nested-ifs.cpp2...
pure2-nested-ifs.cpp2(2,5): error: local variable p must be initialized on both branches or neither branch
pure2-nested-ifs.cpp2(15,13): error: "if" initializes p on:
  branch starting at line 15
but not on:
  branch starting at line 18
  ==> program violates initialization safety guarantee - see previous errors

And the following:

main: (args) = {
    p : *int;

    a := 1;
    b := 2;
    c := 3;
    d := 4;

    if args.cout == 3 {
        p = a&;
    } else if p = b& {
        if args.cout == 2 {
            p = c&;
        } else {
            if b > 0 {
                p = a&;
            } 
            else {
                p = d&;
            }
        }
    } else {
        // p = c&;
    }

    std::cout << p* << std::endl;
}

Gives:

/Users/filipsajdak/dev/execspec/external/tests/pure2-nested-ifs.cpp2...
pure2-nested-ifs.cpp2(2,5): error: local variable p must be initialized on both branches or neither branch
pure2-nested-ifs.cpp2(9,5): error: "if" initializes p on:
  branch starting at line 9
  branch starting at line 11
but not on:
  branch starting at line 22
  ==> program violates initialization safety guarantee - see previous errors

JohelEGP · 2023-04-20T20:46:43Z

    } else if (p.construct(&b)) {
        if (args.cout==2) {
            p.construct(&c);
        }else {
            if (cpp2::cmp_greater(std::move(b),0)) {
                p.construct(&a);
            }
            else {
                p.construct(&d);
            }
        }
    }else {

-- #366 (comment)

This looks wrong. The if is entered if the condition is true, which happens when p is constructed. So the other contruct should be the value accessor to acces the constructed value.
EDIT: It's actually correct. Just not as efficient.

filipsajdak · 2023-04-20T20:49:24Z

Thanks! Definite initialization currently requires that an uninitialized variable be initialized on both branches or neither branch... did you test that this generalizes correctly to requiring that an uninitialized variable be initialized on all branches or no branch?

@hsutter I can now confirm that this generalization correctly requires that an uninitialized variable be initialized on all branches or on no branch.

filipsajdak · 2023-04-20T20:50:04Z

I will add two tests, and it will be ready to merge.

filipsajdak · 2023-04-20T21:26:45Z

OK, tests added. PR ready

filipsajdak · 2023-04-21T00:50:22Z

I need to fix a few cases... it is not yet ready.

hsutter · 2023-04-22T19:41:16Z

Two comments after thinking about this more:

Re if constexpr: Yes, that was not getting emitted. I'll fix this on its own in a separate commit to get it in there. Thanks!

Re else if: I think everything already works correctly, including for guaranteed initialization, except only that else if has to be spelled else { if (and then another } at the end of that block)... correct? If so, I wonder if a less invasive approach would be just to drop the { } requirement if else is followed by if. That would happen during parsing, and wouldn't require any changes in sema.h I think?

Added support for else if branches. Added support for constexpr ifs Added support for definite initialization checks Added support for debug symbols (if branch, if else branch, else branch)

This change allows to initialize variables in `if` and `else if` conditions. It make possible to process the following code: ```cpp main: (args) = { p : *int; a := 1; b := 2; c := 3; d := 4; if args.cout == 3 { p = a&; } else if args.cout == 2 { p = c&; } else if p = b& { p = a&; } else { p = d&; } std::cout << p* << std::endl; } ``` And gets generated: ```cpp auto main(int const argc_, char const* const* const argv_) -> int{ auto args = cpp2::make_args(argc_, argv_); #line 2 "tests/else_if.cpp2" cpp2::deferred_init<int*> p; auto a {1}; auto b {2}; auto c {3}; auto d {4}; if (args.cout==3) { p.construct(&a); } else if (args.cout==2) { p.construct(&c); } else if (p.construct(&b)) { p.value() = &a; } else { p.value() = &d; } std::cout << *cpp2::assert_not_null(std::move(p.value())) << std::endl; } ```

This change brings support for nested `if` or `else if`. It make possible to process the following code: ```cpp main: (args) = { p : *int; a := 1; b := 2; c := 3; d := 4; if args.cout == 3 { p = a&; } else if p = b& { if args.cout == 2 { p = c&; } else { if b > 0 { p = a&; } else { p = d&; } } } else { p = c&; } std::cout << p* << std::endl; } ``` And gets generated: ```cpp auto main(int const argc_, char const* const* const argv_) -> int{ auto args = cpp2::make_args(argc_, argv_); cpp2::deferred_init<int*> p; auto a {1}; auto b {2}; auto c {3}; auto d {4}; if (args.cout==3) { p.construct(&a); } else if (p.construct(&b)) { if (args.cout==2) { p.construct(&c); }else { if (cpp2::cmp_greater(std::move(b),0)) { p.construct(&a); } else { p.construct(&d); } } }else { p.construct(&c); } std::cout << *cpp2::assert_not_null(std::move(p.value())) << std::endl; } ``` Assignements are properly marked as `DEFINITE INITIALIZATION` ``` 0 | function main 1 | scope 2 | var p *** UNINITIALIZED 2 | /var 2 | var a 2 | /var 2 | var b 2 | /var 2 | var c 2 | /var 2 | var d 2 | /var 2 | selection 3 | if branch 4 | *** (10,9) DEFINITE INITIALIZATION OF p 4 | *** use of a 4 | /if branch 3 | *** (11,15) DEFINITE INITIALIZATION OF p 3 | *** use of b 3 | if else branch 4 | selection 5 | if branch 6 | *** (13,13) DEFINITE INITIALIZATION OF p 6 | *** (13,17) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of c 6 | /if branch 5 | else branch 6 | selection 7 | *** (15,16) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of b 7 | if branch 8 | *** (16,17) DEFINITE INITIALIZATION OF p 8 | *** (16,21) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of a 8 | /if branch 7 | else branch 8 | *** (19,17) DEFINITE INITIALIZATION OF p 8 | *** (19,21) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of d 8 | /else branch 7 | /selection 6 | /else branch 5 | /selection 4 | /if else branch 3 | else branch 4 | *** (23,9) DEFINITE INITIALIZATION OF p 4 | *** (23,13) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of c 4 | /else branch 3 | /selection 2 | *** (26,18) DEFINITE LAST POTENTIALLY MOVING USE OF *** use of p 2 | /scope 1 | /function ```

filipsajdak · 2023-04-22T19:56:04Z

@hsutter I have completed the change. Please check the tests that are included.

I have also covered "support for constructing variables in conditions" even though p.construct(&c) cannot be used as a condition as it returns void - cpp1 code generation is fine.

filipsajdak · 2023-04-22T20:05:16Z

I see that I was late ~15min - nice sync. I was doing the last checks and tests. I was thinking about spelling else if as else { if.

I drop that idea for some reason. I will think why as I don't recall it now.

hsutter · 2023-04-22T20:58:51Z

Thanks again for this suggestion and all the work on this.

I'm sorry I have to be the bearer of sad news: I've decided to try out the simpler implementation approach, and coded it up just now. It seems to work well so I'm going to go ahead and check it in. But your work on this has not gone to waste, you raised the issue that we needed else if to work (and /*else*/ if constexpr 😁), you explored and showed one way to do it, and you generated the test cases above that I'm using and will check in for this. Thanks!

Also, your example made me remember that I had forgotten to disallow = initialization inside an if conditional expression. It's already correctly disallowed in other conditional expressions, such as while, because those used logical_expression instead of expression. The if production now does that too.

Also, please check that what I coded up actually resolves this, and reopen this or a new issue if I got it wrong and you spot bugs. Thanks again!

…#366

Also, reject `=` assignment expressions in `if` conditions. They're already disallowed in loop conditions, and it's an oversight that they were not already disallowed in `if` conditions... the `if` condition is supposed to take *logical-or-expression* (the conditional expression production) like the loop conditions do.

filipsajdak changed the title ~~Add support for multiple else ifand if constexpr~~ Add support for multiple else if and if constexpr Apr 13, 2023

JohelEGP mentioned this pull request Apr 14, 2023

[SUGGESTION] Definite first assignment to return this #368

Closed

filipsajdak force-pushed the fsajdak-add-support-for-elseif branch 3 times, most recently from f3a87c7 to afa6878 Compare April 19, 2023 16:36

hsutter reviewed Apr 19, 2023

View reviewed changes

source/sema.h Outdated Show resolved Hide resolved

filipsajdak force-pushed the fsajdak-add-support-for-elseif branch from afa6878 to 853a1e9 Compare April 20, 2023 20:31

filipsajdak marked this pull request as draft April 21, 2023 00:49

filipsajdak force-pushed the fsajdak-add-support-for-elseif branch from 77b48c5 to 8066ef9 Compare April 21, 2023 17:54

hsutter added a commit that referenced this pull request Apr 22, 2023

Actually emit constexpr in if constexpr, partially merges #366

72004cd

filipsajdak force-pushed the fsajdak-add-support-for-elseif branch from 8066ef9 to b502bfc Compare April 22, 2023 19:48

filipsajdak added 2 commits April 22, 2023 21:49

Add support for else if branches and constexpr ifs

565cb54

Added support for else if branches. Added support for constexpr ifs Added support for definite initialization checks Added support for debug symbols (if branch, if else branch, else branch)

filipsajdak added 2 commits April 22, 2023 21:49

Add tests

8fddc94

filipsajdak force-pushed the fsajdak-add-support-for-elseif branch from b502bfc to 8fddc94 Compare April 22, 2023 19:50

filipsajdak marked this pull request as ready for review April 22, 2023 19:51

hsutter closed this in 65bd0a5 Apr 22, 2023

JohelEGP mentioned this pull request May 17, 2023

[DOCS] Assorted comments on the grammar productions #387

Open

zaucy pushed a commit to zaucy/cppfront that referenced this pull request Dec 5, 2023

Actually emit constexpr in if constexpr, partially merges hsutter…

300682d

…#366

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for multiple `else if` and `if constexpr` #366

Add support for multiple `else if` and `if constexpr` #366

filipsajdak commented Apr 13, 2023 •

edited

Loading

JohelEGP commented Apr 13, 2023

filipsajdak commented Apr 13, 2023

filipsajdak commented Apr 13, 2023

filipsajdak commented Apr 13, 2023

filipsajdak commented Apr 13, 2023

JohelEGP commented Apr 13, 2023 •

edited

Loading

JohelEGP commented Apr 14, 2023

hsutter left a comment

filipsajdak commented Apr 19, 2023

filipsajdak commented Apr 19, 2023

filipsajdak commented Apr 20, 2023

filipsajdak commented Apr 20, 2023

JohelEGP commented Apr 20, 2023 •

edited

Loading

filipsajdak commented Apr 20, 2023

filipsajdak commented Apr 20, 2023

filipsajdak commented Apr 20, 2023

filipsajdak commented Apr 21, 2023

hsutter commented Apr 22, 2023

filipsajdak commented Apr 22, 2023

filipsajdak commented Apr 22, 2023

hsutter commented Apr 22, 2023 •

edited

Loading

Add support for multiple else if and if constexpr #366

Add support for multiple else if and if constexpr #366

Conversation

filipsajdak commented Apr 13, 2023 • edited Loading

JohelEGP commented Apr 13, 2023

filipsajdak commented Apr 13, 2023

filipsajdak commented Apr 13, 2023

filipsajdak commented Apr 13, 2023

filipsajdak commented Apr 13, 2023

JohelEGP commented Apr 13, 2023 • edited Loading

JohelEGP commented Apr 14, 2023

hsutter left a comment

Choose a reason for hiding this comment

filipsajdak commented Apr 19, 2023

filipsajdak commented Apr 19, 2023

filipsajdak commented Apr 20, 2023

filipsajdak commented Apr 20, 2023

JohelEGP commented Apr 20, 2023 • edited Loading

filipsajdak commented Apr 20, 2023

filipsajdak commented Apr 20, 2023

filipsajdak commented Apr 20, 2023

filipsajdak commented Apr 21, 2023

hsutter commented Apr 22, 2023

filipsajdak commented Apr 22, 2023

filipsajdak commented Apr 22, 2023

hsutter commented Apr 22, 2023 • edited Loading

Add support for multiple `else if` and `if constexpr` #366

Add support for multiple `else if` and `if constexpr` #366

filipsajdak commented Apr 13, 2023 •

edited

Loading

JohelEGP commented Apr 13, 2023 •

edited

Loading

JohelEGP commented Apr 20, 2023 •

edited

Loading

hsutter commented Apr 22, 2023 •

edited

Loading