Add pointers deduced types

[filipsajdak]

The current implementation has safety guarantees only for explicit pointers declaration. If the pointer type is deduced, cppfront does not provide a safety check.

This change introduces pointer type deduction for types defined in cpp2. After this change cppfront will process below code

fun: (inout i:int) -> *int = {
    return i&;
}

fun2: (inout i:int) -> (result : *int) = {
    result = i&;
}

main: (argc : int, argv : **char) -> int = {
    a:     int = 2;
    pa:   *int = a&;
    ppa: **int = pa&;

    pa = 0;       // caught

    pa2:= ppa*;
    pa2 = 0;      // caught

    pa3 := a&;
    pa3 = 0;      // caught
    pa3 += 2;     // caught

    ppa2 := pa2&;
    pa4 := ppa2*;
    pa4 = 0;      // caught

    pppa := ppa&;
    pa5 := pppa**;
    pa5 = 0;      // caught

    fun(a)++;     // caught
    fp := fun(a);
    fp = 0;       // caught

    f := fun(a)*;

    fp2 := fun2(a).result;
    fp2--;        // not caught :(

    return a * pa* * ppa**; // 8
}

And will generate errors:

pure2-deducing-pointers.cpp2...
pure2-deducing-pointers.cpp2(14,8): error: = - pointer assignment from null or integer is illegal
pure2-deducing-pointers.cpp2(17,9): error: = - pointer assignment from null or integer is illegal
pure2-deducing-pointers.cpp2(20,9): error: = - pointer assignment from null or integer is illegal
pure2-deducing-pointers.cpp2(21,9): error: += - pointer assignment from null or integer is illegal
pure2-deducing-pointers.cpp2(25,9): error: = - pointer assignment from null or integer is illegal
pure2-deducing-pointers.cpp2(29,9): error: = - pointer assignment from null or integer is illegal
pure2-deducing-pointers.cpp2(31,11): error: ++ - pointer arithmetic is illegal - use std::span or gsl::span instead
pure2-deducing-pointers.cpp2(33,8): error: = - pointer assignment from null or integer is illegal
  ==> program violates lifetime safety guarantee - see previous errors
  ==> program violates bounds safety guarantee - see previous errors

Unfortunately, this implementation does not deduce pointer types from functions declared after the analyzed code. That means that defining fun and fun2 functions after the main will block deducing pointer type for lines

fun(a)++;     // not caught anymore
fp := fun(a);
fp = 0;       // not caught anymore

f := fun(a)*; // not deduced as pointer type anymore

This implementation also does not deduce pointer types returned as parameter_declaration_list_node

fp2 := fun2(a).result; // not deduced as pointer
fp2--;        // not caught :(

This replaces: #93 & #94

[hsutter]

Overall, I think if we merge this the work maybe should happen later in sema.h (see comment in the review). But at the moment I'm leaning toward saying that maybe I'd rather leave this until the Lifetime static analysis is implemented, which will do a lot of this Pointer type deduction and state tracking... and that won't be in the near term (sorry!).

I appreciate all the work on this, but I think the best case is that this PR probably should be updated to move some of the analysis to sema, and at that it will be a placeholder that will be replaced... that may limit the amount of additional time you want to put into it at this stage.

Again, sorry, I know this was a lot of work and there are good ideas here, and thanks for understanding!

[hsutter]

I haven't had the cycles to really grok this, and the main reason I'm reading through the PR is because (a) the test case at top is clear and (b) the code change is mostly localized here, both of which are strengths (thanks!). That also limits the downside because it should be very local.

I'm not yet sure whether I should merge this... In the long term I aim to implement my C++ Core Guidelines Lifetime analysis, and that will likely replace at least parts of this work. But if we do merge this now, I still view it as an interim '[expanded] initial partial implementation' and don't think we would want to go further in this direction in the near term. First we would want to implement the Lifetime analysis and then see where we are, and the Lifetime implementation itself needs to be farther in the future after classes and metaclass functions.

[filipsajdak]

The main change here is about calling a new function is_pointer_declaration(unqual->identifier) and to check not only the operator that is called on the node but also the results of the function call (e.g. n()++)

[filipsajdak]

@hsutter Do you think I can try to start implementing a Lifetime static analysis? Or maybe I should try to move pointer reduction to sema? Or do you see anything else I can work on that will support you?

[hsutter]

Thanks. On reviewing this, I think maybe it's okay not to move this to sema -- this would be a (useful) placeholder, but something that would be replaced in the future and is pretty localized now. So I would lean toward taking this pretty much as is, and not trying to go further on this path for now.

I appreciate the offer for implementing Lifetime, but that will go best after classes are done because parts of Lifetime depend on classes. Probably similar with most other feature extension PRs -- I think the most useful thing will be for me to focus on classes in the coming few months, and in the meantime if you can focus on letting me know about any bug reports and repros that you find that would be useful (thanks again for those and please send those anytime!), and then after classes are in and stable it would be time to look at new/extended features again.

[hsutter]

Thanks for including a new regression test.

Minor naming thing that I can clean up post-merge: Tests that are intended to fail cppfront compilation (not generate a .cpp file) should have a name that ends in ...error.cpp2. That way my super-simple run-tests.bat test runner will count it as not intending to to create a .cpp and not flag a mismatch error. I'll take care of this post-merge though.

[filipsajdak]

Right... will correct that.

[filipsajdak]

Oh, I see that you have merged that. Should I prepare another PR with the name change?

[filipsajdak]

By the way. Thank you for merging that. Appreciate it!

[hsutter]

Yup, I did in another commit. All's well, thanks!