Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] Malformed lambda call inside inspect lead to compilation without error and silent default value #433

Open
realgdman opened this issue May 7, 2023 · 9 comments · May be fixed by #529
Labels
bug Something isn't working

Comments

@realgdman
Copy link

realgdman commented May 7, 2023

Inside inspect, I have lambda where I forgot to pass argument. Like
:(x) -> _ = {return "int"} (); instead of
:(x) -> _ = {return "int"} (x);
It was accepted by compilers and emitted code, which runs and silently returns ""

To Reproduce
https://cpp2.godbolt.org/z/aerezGKqq

Reproduction code

gettype: (x: _) -> std::string = { 
  return (inspect x -> std::string {
    is int = :(x) -> _ = { std::cout<<x<<":"; return "int";} ();  //forgot to pass argument to lambda
  //is int = :(x) -> _ = { std::cout<<x<<":"; return "int";} (x); //ok, this would output  "13:int, other"

    is _ = "other";
  });
}

main: () = {
  std::cout << gettype(13) << ", " << gettype(false) << std::endl;	
  //outputs ", other" instead of diagnostic
    
  // :(x) -> _ = { std::cout<<x<<":"; return "int";} ();  //correct cpp1 error diagnostic
}

Command line
cppfront/cppfront $1.cpp2 -p
clang++-15 -Icppfront/include $1.cpp -std=c++20 -o $1

Expected result
Diagnostic like cpp1 for line 13:

error: no matching function for call to object of type '(lambda at lambdaparam.cpp2:13:6)'
note: candidate function template not viable: requires single argument 'x', but no arguments were provided

Actual result/error
Code silently compiles at cpp2 and cpp1 and silently runs without calling lambda and returning default value for inspect case, in this case an empty string.

Additional context
I understand this may be alpha version limitation of inspect

Emitted cpp1 code is roughly
if (cpp2::is(__expr)) { if constexpr( requires{V} ) if constexpr( std::is_convertible_v<V,T>) return V; else return T{}; else return T{}; }

Edit: after some investigation, it appears that cpp1
if constexpr( requires{[](auto const& x) -> auto{ ... }();} ) returns false.

@realgdman realgdman added the bug Something isn't working label May 7, 2023
@JohelEGP
Copy link
Contributor

JohelEGP commented May 7, 2023

The problem are those trailing

        else
          return std::string{};
      else
        return std::string{};

I think they should be replaced with a contract check or something along those lines.
The user wrote a statement for the chosen alternative, but it doesn't work for it!

@JohelEGP
Copy link
Contributor

JohelEGP commented May 7, 2023

See https://compiler-explorer.com/z/MKTTxoTn7.
I replaced the above with an assertion,
and dropped the else generated from is _ -> to return unconditionally in order to silence warnings.

Program returned: 139
output.s: /app/example.cpp:49: gettype<int>(const int&)::<lambda()>: Assertion `false && "Ill-formed statement for chosen alternative!"' failed.
Program returned: 139
output.s: /app/example.cpp:49: auto gettype(const int &)::(anonymous class)::operator()() const: Assertion `false && "Ill-formed statement for chosen alternative!"' failed.

@realgdman
Copy link
Author

Assert is good but I'm surprised that cpp1 if constexpr( requires{ eats non-compilable code. Probably this can be addressed with some extra check.

@JohelEGP
Copy link
Contributor

JohelEGP commented May 7, 2023

It's basically std::invocable<closure_type> rather than std::invocable<closure_type, int>,
so I think it's OK the branch is discarded.

@JohelEGP
Copy link
Contributor

JohelEGP commented May 8, 2023

Folded the is-convertible check into the requires expression: https://compiler-explorer.com/z/zvxn7Mea6.

@filipsajdak
Copy link
Contributor

This is more complicated than that. These multiple constexpr ifs are there to eliminate cases when the evaluation of a branch is invalid for a specific inspected value/type.

I need to check if asserts will work for complex cases. For sure static_asserts were not an option.

@JohelEGP
Copy link
Contributor

JohelEGP commented May 8, 2023

This single branch should be enough for an alternative's statement:

if constexpr(requires { { 𝘦𝘹𝘱𝘳𝘦𝘴𝘴𝘪𝘰𝘯 } -> std::convertible_to<T>; })
  return 𝘦𝘹𝘱𝘳𝘦𝘴𝘴𝘪𝘰𝘯;

Followed by some kind of assertion in case the branch was discarded for the chosen alternative.

@realgdman
Copy link
Author

(Sorry I haven't tried any fixes)

Another observation, currently it's impossible to capture in lambda inside inspect

S: type = {
  operator=: (out this) = {}
  operator=: (out this, that) = {}
  print: (this) = std::cout << "S"; 
}

main: () -> int = {
  s := S();
    
  l := :() -> int = { (s$ as S).print(); return 1; };  //ok
  l();
    
  tmp := (inspect s -> int {
  //is S = l(); //works
    is S = :() -> int = { (s$ as S).print(); return 1; }(); //error
    is _ = 0; 
  });
}

https://cpp2.godbolt.org/z/MGbxfTd6z

l can capture s, but exact same lambda inside inspect cannot.
Same with s&$*
Cpp1 Error message is
error: reference to local variable 's' declared in enclosing function 'main'
As far as I can tell it points at
if constexpr( requires{[_0 = (&s)]() ....

@JohelEGP
Copy link
Contributor

JohelEGP commented Jun 3, 2023

Another observation, currently it's impossible to capture in lambda inside inspect

Please, open a separate issue for that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
3 participants