[BUG] Explicit discard requirement may be broken?

[bluetarpmedia]

Describe the bug A recent commit may have broken the explicit discard requirement.

To Reproduce Run cppfront on this code:

m: std::mutex = ();    // <--- Never used again

increment: (inout x) = { x++; }

main: () = {
    v1: std::vector = (11, 22, 33);
    sz:= v1.size();    // <--- Never used again

    value:= 42;
    increment(value);    // <--- inout but never used again
}

There are no errors generated. Repro on Godbolt which at the time of writing is up-to-date with cppfront's main branch (Build 9318:2141).

[gregmarr]

Something might have updated, because it's erroring now (the error is from clang, not cppfront):

main.cpp2:10:5: error: no matching function for call to 'increment'
   10 |     increment(cpp2::move(value));
      |     ^~~~~~~~~
main.cpp2:3:6: note: candidate function [with x:auto = int] not viable: expects an lvalue for 1st argument
    3 | auto increment(auto& x) -> void{++x; }
      |      ^         ~~~~~~~

Fixing that by adding _ = value; results in:

main.cpp2:7:10: warning: unused variable 'sz' [-Wunused-variable]
    7 |     auto sz {CPP2_UFCS(size)(cpp2::move(v1))}; 
      |          ^~

[bluetarpmedia]

Something might have updated, because it's erroring now (the error is from clang, not cppfront):

Yeah, I get subsequent warnings/errors from the compiler, but I'm pretty sure in previous versions of cppfront that cppfront itself produced an error about having to discard the variable.

[gregmarr]

I'm pretty sure in previous versions of cppfront that cppfront itself produced an error about having to discard the variable.

Could be, I don't remember one way or the other.

[hsutter]

Thanks! I don't think the first two cases were ever diagnosed:

m: std::mutex = ();    // <--- Never used again

This one is not a discard situation (there's no return/out value being discarded). Also it's a global, and the use rules were only for local values.

increment: (inout x) = { x++; }

main: () = {
    v1: std::vector = (11, 22, 33);
    sz:= v1.size();    // <--- Never used again

This is also not a discard situation, it's an unused variable... however, the Cpp1 compiler should diagnose it, if "unused local object" warnings are enabled.

    value:= 42;
    increment(value);    // <--- inout but never used again
}

This is a discard situation and I just tried it and it's caught as intended, because this last use of value is lowered to move(value) and then the call refuses to bind to the parameter (which is lowered as non-const & and so requires an lvalue), so we get a diagnostic due to the fact that the "out" value is discarded.

So I think this is okay, and I'll close the issue. Let me know if you think I missed anything, and thanks again.