hsutter / cppfront

A personal experimental C++ Syntax 2 -> Syntax 1 compiler
Other
5.24k stars 223 forks source link

[BUG] Last use of member function cannot be inout this #999

Closed ProgrammingRainbow closed 3 months ago

ProgrammingRainbow commented 4 months ago

I think this is similar to this bug https://github.com/hsutter/cppfront/issues/888 because i believe it's to do with the implicit move.

If the second to last member function is (inout this) the code runs as expected. But if you comment out or remove the "x.print();" making "x.inc();" that is an (inout this) the last use of x. then it falls apart.

myclass : type = {
    data: int = 42;
    more: std::string = std::to_string(42);

    // method
    print: (this) = {
        std::cout << "data: " << data << ", more: " << more << std::endl;
    }

    // non-const method
    inc: (inout this) = {
        data++;
    }
}

main: () = {
    x: myclass = ();
    x.inc();
    x.print();
}
❯ ../cppfront test3.cpp2
test3.cpp2... ok (all Cpp2, passes safety checks)

❯ g++ -I../include -std=c++23 test3.cpp -o test3

❯ ./test3
data: 43, more: 42
myclass : type = {
    data: int = 42;
    more: std::string = std::to_string(42);

    // method
    print: (this) = {
        std::cout << "data: " << data << ", more: " << more << std::endl;
    }

    // non-const method
    inc: (inout this) = {
        data++;
    }
}

main: () = {
    x: myclass = ();
    x.inc();
}
❯ ../cppfront test3.cpp2
test3.cpp2... ok (all Cpp2, passes safety checks)

❯ g++ -I../include -std=c++23 test3.cpp -o test3
test3.cpp2: In function ‘int main()’:
test3.cpp2:18:19: error: no match for call to ‘(main()::<lambda(Obj&&, Params&& ...)>) (std::remove_reference<myclass&>::type)’
   18 |     x.inc();
In file included from test3.cpp:6:
../include/cpp2util.h:927:1: note: candidate: ‘template<class Obj, class ... Params, bool IsNothrow> main()::<lambda(Obj&&, Params&& ...)>’
  927 | [LAMBDADEFCAPT]< \
      | ^
../include/cpp2util.h:942:59: note: in expansion of macro ‘CPP2_UFCS_’
  942 | #define CPP2_UFCS(...)                                    CPP2_UFCS_(&,CPP2_UFCS_IDENTITY,(),,__VA_ARGS__)
      |                                                           ^~~~~~~~~~
test3.cpp2:18:5: note: in expansion of macro ‘CPP2_UFCS’
   18 |     x.inc();
      |     ^~~~~~~~ 
../include/cpp2util.h:927:1: note:   template argument deduction/substitution failed:
  927 | [LAMBDADEFCAPT]< \
      | ^
../include/cpp2util.h:942:59: note: in expansion of macro ‘CPP2_UFCS_’
  942 | #define CPP2_UFCS(...)                                    CPP2_UFCS_(&,CPP2_UFCS_IDENTITY,(),,__VA_ARGS__)
      |                                                           ^~~~~~~~~~
test3.cpp2:18:5: note: in expansion of macro ‘CPP2_UFCS’
   18 |     x.inc();
      |     ^~~~~~~~ 
../include/cpp2util.h:927:1: note: constraints not satisfied
  927 | [LAMBDADEFCAPT]< \
      | ^
../include/cpp2util.h:942:59: note: in expansion of macro ‘CPP2_UFCS_’
  942 | #define CPP2_UFCS(...)                                    CPP2_UFCS_(&,CPP2_UFCS_IDENTITY,(),,__VA_ARGS__)
      |                                                           ^~~~~~~~~~
test3.cpp2:18:5: note: in expansion of macro ‘CPP2_UFCS’
   18 |     x.inc();
      |     ^~~~~~~~ 
test3.cpp2: In substitution of ‘template<class Obj, class ... Params, bool IsNothrow> main()::<lambda(Obj&&, Params&& ...)> [with Obj = myclass; Params = {}; bool IsNothrow = false]’:
test3.cpp2:18:19:   required from here
test3.cpp2:18:5:   required by the constraints of ‘template<class Obj, class ... Params, bool IsNothrow> main()::<lambda(Obj&&, Params&& ...)>’
../include/cpp2util.h:916:1: note: no operand of the disjunction is satisfied
  915 |    requires { CPP2_FORWARD(obj).CPP2_UFCS_REMPARENS QUALID TEMPKW __VA_ARGS__(CPP2_FORWARD(params)...); } \
      |    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  916 | || requires { MVFWD(CPP2_UFCS_REMPARENS QUALID __VA_ARGS__)(CPP2_FORWARD(obj), CPP2_FORWARD(params)...); }
      | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
../include/cpp2util.h:934:14: note: in expansion of macro ‘CPP2_UFCS_CONSTRAINT_ARG’
  934 |     requires CPP2_UFCS_CONSTRAINT_ARG(MVFWD,QUALID,TEMPKW,__VA_ARGS__) { \
      |              ^~~~~~~~~~~~~~~~~~~~~~~~
../include/cpp2util.h:942:59: note: in expansion of macro ‘CPP2_UFCS_’
  942 | #define CPP2_UFCS(...)                                    CPP2_UFCS_(&,CPP2_UFCS_IDENTITY,(),,__VA_ARGS__)
      |                                                           ^~~~~~~~~~
test3.cpp2:18:5: note: in expansion of macro ‘CPP2_UFCS’
   18 |     x.inc();
      |     ^~~~~~~~ 
cc1plus: note: set ‘-fconcepts-diagnostics-depth=’ to at least 2 for more detail
hsutter commented 3 months ago

Thanks! I think this is resolved by 70419944c4ba9166ac6b479cd6eef0ca410235a3, by making the error message better.

It's actually intentional that this is an error. Briefly, the reason is that if a definite last use of a local object modifies that object, then the code is not looking at the object again (because by definition it's a last use) and therefore trying to implicitly discard the new value. For a complete discussion, please see Design note: Explicit discard.

However, you are correct that the error message above is terrible. Fortunately, I just fixed that in commit 70419944c4ba9166ac6b479cd6eef0ca410235a3... now for the latter program above, the diagnostic is much better, it starts with this:

demo.cpp2(18): error C2338: static_assert failed: 'this function call syntax tries 'obj.func(...)', then 'func(obj,...);' - both failed, but the first would have succeeded if obj were an lvalue - the likely problem is that this is a definite last use of the object (which automatically treats it as an rvalue / move candidate), and the function cannot accept an rvalue'

Followed by semi-useful information (showing MSVC output here):

demo.cpp2(18): note: the template instantiation context (the oldest one first) is
demo.cpp2(18): note: see reference to function template instantiation 'decltype(auto) main::<lambda_1>::operator ()<myclass,,false>(Obj &&) noexcept(false) const' being compiled
        with
        [
            Obj=myclass
        ]
demo.cpp2(18): error C2662: 'void myclass::inc(void) &': cannot convert 'this' pointer from 'Obj' to 'myclass &'
        with
        [
            Obj=myclass
        ]
demo.cpp2(18): note: A non-const reference may only be bound to an lvalue
demo.cpp2(11): note: see declaration of 'myclass::inc'
demo.cpp2(18): note: while trying to match the argument list '()'
demo.cpp2(18): error C3861: 'inc': identifier not found
demo.cpp2(18): note: 'inc': function was not declared in the template definition context and can be found only via argument-dependent lookup in the instantiation context
hsutter commented 3 months ago

However, based on this feedback I think changing the first error message as follows would be clearer about the actual problem, and advise how to fix it:

demo.cpp2(18): error C2338: static_assert failed: 'error: implicit discard of an object's modified value is not allowed - this function call modifies 'obj', but 'obj' is never used again in the function so the new value is never used - if that's what you intended, add another line '_ = obj;' afterward to explicitly discard the new value of the object'

Thanks!

bluetarpmedia commented 3 months ago

I'm not convinced about the semantics of explicit discard in this situation.

I'm thinking about RAII-style types, e.g.:

raii: type = {
    operator=: (out this) = {
        f = nullptr;
        // Pretend this is: fopen(...);
    }
    operator=: (move this) = {
        // Pretend this is: fclose(...);
    }
    write: (inout this, something: int) = {
        // Pretend this is: fwrite(...);
    }
    f: *FILE;
}

main: () -> int = {

    {
        file: raii = ();
        file.write(55);
    }

    return 0;
}

I'd find it awkward to have to write _ = file to produce a valid program -- coming from the Cpp1 perspective where RAII behaviour is well established.

hsutter commented 3 months ago

OK, reopened. See also my comment on #1002 which also came down to "this is about guard objects I think"...

hsutter commented 3 months ago

Thanks! See #1030, this should now be fixed in the above commit.

Both code examples above will now work.