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] Cannot value initialise a function argument with equivalent of C++ {} #1090

Closed
bluetarpmedia opened this issue May 30, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@bluetarpmedia
Copy link
Contributor

Describe the bug
I can't find a Cpp2 syntax to value initialise an argument passed to a function using its default constructor.

To Reproduce
Run cppfront on this code:

log: (message: std::string_view,
      context: std::string,
      event_id: int) =
{
    std::println("{} {} {}", message, context, event_id);
}

main: () = {
    log("event", (), 123);  // (1) Error in C++ compiler
    log("event", _, 123);   // (2) Error in C++ compiler
    log("event", {}, 123);  // (3) Error in cppfront
}

(1) produces this error:

main.cpp2:10:19: error: expected expression
   10 |     log("event", (), 123);
      |                   ^

(2) produces this error:

main.cpp2:10:18: error: use of undeclared identifier '_'
   10 |     log("event", _, 123);
      |                  ^

(3) produces a cppfront syntax error.

Repro on Godbolt

Additional context
I was translating the ranges::find_end example from cppreference and have used explicit arguments like this:

found4:= std::ranges::find_end(
    secret, "SWORD"sv,
    std::ranges::equal_to(),
    std::identity(),
    :(c: char) std::tolower(c));

instead of the original:

const auto found4 = std::ranges::find_end(secret, "SWORD"sv,
        {},
        {},
        [](char c) { return std::tolower(c); });
@bluetarpmedia bluetarpmedia added the bug Something isn't working label May 30, 2024
@sookach
Copy link
Contributor

sookach commented May 30, 2024

Hi Neil,
The solution I think might be simplest for this is to use '{}' for default initialized arguments just like c++, and adding it would be pretty straight forward. One approach is to allow brace expressions here:

cppfront/source/parse.h

Lines 5715 to 5718 in 130f149

if (curr().type() == lexeme::LeftParen
// If in the future (not now) we decide to allow braced-expressions
// || curr().type() == lexeme::LeftBrace
)

However that approach might not be best one given the comment right above it. I can probably add logic to the parser to grab '{}' only in this specific instance, but I'd like to hear from our BDFL, @hsutter, before going down any route.

Thanks.

@hsutter
Copy link
Owner

hsutter commented May 30, 2024

Thanks! In Cpp2 so far, { } are used only for scopes.

Hot take: () does already mean default construction (e.g., x : int = ();) so an empty list argument () could be lowered to {}.

@sookach
Copy link
Contributor

sookach commented May 30, 2024

I like that idea, I'll try it out.

@DerpMcDerp
Copy link

if () is changed to lower to {} then wouldn't that imply that explicit constructors should be included in the overload set? i.e.

The following doesn't work in C++ (but should)

struct B {
	explicit B(int);
};

void foo(B);

B asdf() {
	B _ = auto{1}; // should mean B _ = B{1};
	foo({1});      // should mean foo(B{1}); (some might disagree)
	foo(auto{1});  // should mean foo(B{1});

	if (rand())
		return {1};     // should mean return B{1}; (some might disagree)
	else
		return auto{1}; // should mean return B{1};
}

The following should work in Cpp2:

B: type = {
	operator=: (out this, _: i32) = {} // explicit ctor
}

foo: (_: B) = {}

B asdf() {
	_: B = (1);
	foo((1)); // some might disagree that this should work
	foo(:_ = (1));
	foo(_(1));

	if (rand())
		return (1); // some might disagree that this should work
	else if (rand())
		return :_ = (1);
	else
		return _(1);
}

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
Development

No branches or pull requests

4 participants