[SUGGESTION][FIX] Add safety check support for cpp1 pointers and deduced from other cpp2 declarations

[filipsajdak]

The current implementation has no safety checks for:

• deduced pointers from cpp1 or functions that deduce return types,
• deduced pointers from cpp2 variables or functions,

This change brings support for these cases and makes the below code fail to compile due to safety check errors:

#include <memory>
#include <array>

int* to_pointer(int& i) {
    return &i;
}

std::unique_ptr<int> to_up(int i) {
    return std::make_unique<int>(i);
}

auto to_array() {
    std::array<int, 64> out;
    for(int i = 0; i < std::ssize(out); ++i)
        out[i] = i;
    return out;
}

to_pointer2: (inout i: int) -> auto = {
    return i&;
}

to_value: (inout i: int) -> int = {
    return i;
}

fun_ptr: (p : *int) -> int = {
    return p*;
}

int   g = 44;
int* pg = &g;

main: () -> int = {
    i : int = 42;

    pi  := to_pointer(i);
    pi2 := to_pointer2(i);

    pi   = 0; // caught on cpp1 side
    pi2 += 1; // caught on cpp1 side

    l := :(inout x : _) = x&;

    lpg := pg;

    lpg = 0;  // caught on cpp1 side

    ri := i;
    ri2 := to_value(i);

    up := to_up(123);

    ar := to_array();

    v := fun_ptr(to_pointer(i));
}

cppfront will identify which initializations are suspicious and put them into the cpp2::safety_check() function. Suspicious initialization is initialization by a function or identifier that cppfront has no information about (i.e. don't know the return type or the variable is from cpp1 and we have no sema information about it).

The function checks if it deals with the pointer type. If it is then the pointer is packed into cpp2::safetychecked_pointer that has static_asserts that check if lifetime safety guarantee rules are followed by overloading most operators.

The above code will compile to:

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

#line 1 "../tests/pointer-from-cpp1.cpp2"
#include <memory>
#include <array>

int* to_pointer(int& i) {
    return &i;
}

std::unique_ptr<int> to_up(int i) {
    return std::make_unique<int>(i);
}

auto to_array() {
    std::array<int, 64> out;
    for(int i = 0; i < std::ssize(out); ++i)
        out[i] = i;
    return out;
}

[[nodiscard]] auto to_pointer2(int& i) -> auto;
#line 23 "../tests/pointer-from-cpp1.cpp2"
[[nodiscard]] auto to_value(int& i) -> int;
#line 27 "../tests/pointer-from-cpp1.cpp2"
[[nodiscard]] auto fun_ptr(cpp2::in<int *> p) -> int;
#line 30 "../tests/pointer-from-cpp1.cpp2"

int   g = 44;
int* pg = &g;

[[nodiscard]] auto main() -> int;

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

#line 18 "../tests/pointer-from-cpp1.cpp2"

[[nodiscard]] auto to_pointer2(int& i) -> auto{
    return &i; 
}

[[nodiscard]] auto to_value(int& i) -> int{
    return i; 
}

[[nodiscard]] auto fun_ptr(cpp2::in<int *> p) -> int{
    return *p; 
}
#line 33 "../tests/pointer-from-cpp1.cpp2"

[[nodiscard]] auto main() -> int{
    int i { 42 }; 

    auto pi { cpp2::safety_check(to_pointer(i)) }; 
    auto pi2 { to_pointer2(i) }; 

    pi   = 0; // caught on cpp1 side
    pi2 += 1; // caught on cpp1 side

    auto l { [](auto& x) { return &x; } }; 

    auto lpg { cpp2::safety_check(pg) }; 

    lpg = 0;  // caught on cpp1 side

    auto ri { i }; 
    auto ri2 { to_value(i) }; 

    auto up { cpp2::safety_check(to_up(123)) }; 

    auto ar { cpp2::safety_check(to_array()) }; 

    auto v { fun_ptr(cpp2::safety_check(to_pointer(i))) }; 
}

When you will try to compile it with the cpp1 compiler you will end with an error:

% clang++ -std=c++20 -Iinclude/ ../tests/pointer-from-cpp1.cpp        
In file included from ../tests/pointer-from-cpp1.cpp:2:
include/cpp2util.h:568:48: error: static_assert failed due to requirement 'program_violates_lifetime_safety_guarantee<int>' "pointer assignment from integer is illegal"
    constexpr void operator=(X lhs) noexcept { static_assert(program_violates_lifetime_safety_guarantee<X>, "pointer assignment from integer is illegal"); }
                                               ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../tests/pointer-from-cpp1.cpp2:22:10: note: in instantiation of function template specialization 'cpp2::safetychecked_pointer<int *>::operator=<int>' requested here
    pi   = 0; // caught on cpp1 side
         ^
In file included from ../tests/pointer-from-cpp1.cpp:2:
include/cpp2util.h:530:54: error: static_assert failed due to requirement 'program_violates_lifetime_safety_guarantee<int>' "pointer arithmetic is illegal - use std::span or gsl::span instead"
    template <typename X> void operator+= (X) const {static_assert(program_violates_lifetime_safety_guarantee<X>, "pointer arithmetic is illegal - use std::span or gsl::span instead");}
                                                     ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../tests/pointer-from-cpp1.cpp2:23:9: note: in instantiation of function template specialization 'cpp2::safetychecked_pointer<int *>::operator+=<int>' requested here
    pi2 += 1; // caught on cpp1 side
        ^
2 errors generated.

Not all errors are listed at once - as the compilation process stops on errors. Errors are done in a way that they mimic errors that comes from cppfront:

'program_violates_lifetime_safety_guarantee<int>' "pointer arithmetic is illegal - use std::span or gsl::span instead"

Update 2022-10-30

After moving pointer decorators to unqualified_id_node we can now deduce the types based on a declaration of variables and functions that are used to initialize the variable.

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

main: () -> int = {
    i := 42;
    p := fun(i);

    fun(i)++;  // captured
    fun(i)--;  // captured
    fun(i)[0]; // captured
    fun(i)~;   // captured

    p = 0; // captured
    p++;   // captured
    p--;   // captured
    p[0];  // captured
    p~;    // captured

    fun(i)* = 0; // works
    p* = 42;     // works
}

---

Link to previously used working prototypes:

• current tests: https://godbolt.org/z/4n5Yraend
• previous implementation with macro: https://godbolt.org/z/Gc8K4vnrM

[filipsajdak]

Forget to add link to working prototype: https://godbolt.org/z/Gc8K4vnrM

[filipsajdak]

This change has more issues (e.g. pointers to functions, moving of lvalue, or problem with lack of closing parentheses when initialization happens together with UFCS).

I already have a fix. Will push them soon.

[filipsajdak]

The last push solves all issues I have found. Also brings support for deducing pointer types of the cpp2 functions or objects.

After this change cppfront will be able to capture safety violations presented below:

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

main: () -> int = {
    i := 42;
    p := fun(i);

    fun(i)++;  // captured
    fun(i)--;  // captured
    fun(i)[0]; // captured
    fun(i)~;   // captured

    p = 0; // captured
    p++;   // captured
    p--;   // captured
    p[0];  // captured
    p~;    // captured
}

Thanks to better introspection of the cpp2 functions and variables that are known to cppfront (and which return type is explicitly stated). Variables that are initialized using cpp2 variables are known and it is clear to cppfront if it is a pointer or not. That means that fewer functions and variables will be surrounded by cpp2::safety_check() (and will be checked by the cpp1 compiler).

Also, function pointers are now secured with cpp2::safety_check(). If you will try to use carray you will also get an error:

pointer arithmetic is illegal - use std::span or gsl::span instead

[filipsajdak]

While working on this PR I found a bug in the get_declaration_of() method - sometimes it blocks deduction of the type. Issue is solved here: https://github.com/hsutter/cppfront/pull/102 and will be needed for this PR to work in all cases.

[filipsajdak]

As I reworked handling pointer decorators this PR replaces implementation from: https://github.com/hsutter/cppfront/pull/99

I leave both as one https://github.com/hsutter/cppfront/pull/99 is simple and can be used without this big PR.

[filipsajdak]

@hsutter in https://github.com/hsutter/cppfront/pull/102 you renamed the function get_declaration_of to get_local_declaration_of. In this PR, I have added a parameter to the old function that changes the behavior of the function to look outside of the named function.

My code looks like this:

auto get_declaration_of(token const& t, bool look_beyond_current_function = false) -> declaration_sym const*
{
// skipping some code
        if (
            decl.declaration->type.index() == declaration_node::function //  Don't look beyond the current function
            && !look_beyond_current_function
        ) {
            return nullptr;
        }
// skipping some code
)

Now, it does not make sense to add such a flag to the get_local_declaration_of - the name already explains that it is all about local declarations. Should I rename the function and use my version with the flag, or should I create a new function?

[hsutter]

See #93 comment... but a note specifically about * pointer-declarators, those should move to type_id_node.

[filipsajdak]

Yes, I know. I was trying to move it. I will make that change.

[hsutter]

Thanks again for this. It's a big PR and it would take me a lot of time to review, and perhaps the implementation will be easier a few months from now that we have a clearer type-id node and soon actual types.

So I'm going to close this for now, but if you still feel you want to pursue it perhaps we could revisit this in the summer with fresh PR? Thanks for understanding, and thanks again for all your contributions!

[filipsajdak]

I was going to close it too.

[hsutter]

@filipsajdak Thanks. Same for #196?

[filipsajdak]

@hsutter I have ported #196 to the type_id node and simplified the algorithm for deducing pointers.

Could you take a look at it? It deduces pointers pretty well, and if it does not collide with your other work, cppfront will benefit from having it working.

Of course, if it collides, do not hesitate to postpone it.