cppfront icon indicating copy to clipboard operation
cppfront copied to clipboard

[FIX] Add support for multi-level pointers

Open filipsajdak opened this issue 3 years ago • 5 comments

Current implementation supports only pointer of the form

p: *int = ...;

This change extends that to allow multi-level pointers and make below code compilable

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

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

and generates the cpp1 code

// ----- Cpp2 support -----
#include "cpp2util.h"


#line 1 "tests/pointer-to-pointer.cpp2"
[[nodiscard]] auto main(cpp2::in<int> argc, cpp2::in<char * *> argv) -> int;

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

#line 1 "tests/pointer-to-pointer.cpp2"
[[nodiscard]] auto main(cpp2::in<int> argc, cpp2::in<char * *> argv) -> int{
    int a { 2 }; 
    int* pa { &a }; 
    int** ppa { &pa }; 

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

Closes https://github.com/hsutter/cppfront/issues/78 and makes possible to implement correct main function with arguments.

Open issues

This change still does not support deduced pointer types (just like the current implementation). That means:

pa = 0;

will cause an error as cppfront knows that pa is a pointer to pointer

tests/pointer-to-pointer.cpp2...
pointer-to-pointer.cpp2(6,8): error: = - pointer assignment from null or integer is illegal
  ==> program violates lifetime safety guarantee - see previous errors

but this code:

pa2 := ppa*;
pa2 = 0;

pa3 := a&;
pa3 = 0;

will pass lifetime safety guarantee checks and will compile to cpp1 code:

auto pa2 { *ppa }; 
pa2 = 0;

auto pa3 { &a }; 
pa3 = 0;

Having multi-level pointer makes that issue more important to be fixed as it is easier to deduce a pointer from pointer to pointer.

filipsajdak avatar Oct 24 '22 14:10 filipsajdak

On the second thought, I am also not using index so nevertheless the range-for is better

for (auto _ : n.pointer_declarators) {
    printer.print_cpp2("*", n.position());
}

I was thinking of using std::ignore but it is not valid in this context. I pushed the corrected version.

filipsajdak avatar Oct 24 '22 15:10 filipsajdak

I just spend couple of minutes meditating @hsutter comment (that I have modified for this change)

//  TODO: Generalize this -- for now we detect only multi-level cases of the form "p: ***int = ...;"
//        We don't recognize pointer types that are deduced or from Cpp1

and I did some tests that makes me add Open issues to the description of this PR as this needs to be clear that this PR introduce support for multi-level pointers but does not address other issues. One of them: deduced pointers becomes more an issue after this PR as it will be easier to create a deduced pointer from pointer to pointer.

filipsajdak avatar Oct 24 '22 20:10 filipsajdak

Handling of pointer deduced types are done here: https://github.com/hsutter/cppfront/pull/94

filipsajdak avatar Oct 25 '22 01:10 filipsajdak

Pointers from cpp1 are handled here https://github.com/hsutter/cppfront/pull/96

filipsajdak avatar Oct 27 '22 13:10 filipsajdak

Add support for pointers as template arguments

s : std::span<*char> = (argv, gsl::narrow_cast<std::size_t>(argc)); // works

filipsajdak avatar Nov 01 '22 22:11 filipsajdak

@filipsajdak Would you be interested in updating the good/current parts of #93, #94, and #96 and merging them into a new PR to replace these?

hsutter avatar Dec 26 '22 22:12 hsutter

Yes. I will.

filipsajdak avatar Dec 26 '22 23:12 filipsajdak

Replaced by https://github.com/hsutter/cppfront/pull/196

filipsajdak avatar Jan 03 '23 22:01 filipsajdak