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] order-independence of functions fails when using default arguments #75

Closed
neumannt opened this issue Oct 14, 2022 · 10 comments
Closed
Labels
bug Something isn't working

Comments

@neumannt
Copy link

While implementing semantic analysis I noticed a conceptual problem with the order-independence approach of cpp2. Consider this code here:

foo: (x : int = bar()) -> int = { return 3*x+1; }
bar: (x : int = 2) -> int = { return x/2; }
main: () -> int = { return foo(); }

cppfront happily accept that code, but it then fails in C++ because the order of forward declarations is wrong. Swapping the order of foo and bar fixes that problem. (Modulo another bug that the default argument is repeated at the declaration site, which also makes the C++ compiler unhappy).

One question is, what should the intended semantic be? Can default arguments reference functions that are further down in the translation unit? Intuitive, if we fully bet on order-independence, the answer should be yes. On the other hand implementing that is actually quite subtle. Note that it is not always possible find a topological order, for example here:

foo: (x : int = bar()) -> int = { return 3*x+1; }
bar: (x : int = foo()) -> int = { return x/2; }
main: () -> int = { return foo(); }

Clearly that must trigger an error. We could detect this cyclic dependency, but it increases the complexity of the compiler. Another question is, is the compiler required to analyze default arguments if they are never evaluated? If not, we could simply translate them lazily on demand and check for cycles during expansion. (Though lowering that to C++ will be nightmare, this approach will only work of default arguments are expanded by cppfront itself and not by the underlying C++1 compiler).

Note that we will have exactly the same problem with auto return types. These also require analyzing the function itself, which might depend on other functions further down.

I see basically two options. Either we forbid this, requiring that functions are defined before they are used as default arguments or with auto return types. Or we make function analysis lazy, which requires the compiler to topological order functions according to their dependencies, producing an error if that is not possible.

@neumannt neumannt added the bug Something isn't working label Oct 14, 2022
@hsutter
Copy link
Owner

hsutter commented Dec 26, 2022

Thanks!

cppfront happily accept that code

Good point, it should reject the default argument. I've been meaning to add that check but never did, I'll do it now.

And it's for exactly the reason you give: Default arguments (and deduced return types) are order-dependent. Currently I have deduced return types as the default only for local (unnamed) functions, which is fine; I guess I could similarly allow default arguments on local functions, but that would actually be an additional explicitly visible feature and so my current gut feeling is to wait to see if there's even demand for it.

Either we forbid this, requiring that functions are defined before they are used as default arguments or with auto return types.

+1

Or we make function analysis lazy, which requires the compiler to topological order functions according to their dependencies, producing an error if that is not possible.

That's not only a lot more work to implement, but I worry it could be really brittle and likely make it harder for programmers to reason about their code. For example, it would be bad if changing the implementation of one function could cause other possibly-distant functions that used to work to suddenly fail to compile... that would be against encapsulation (exposing implementation detail) and composability. It's really important that callers should depend on stable interfaces only, that's composable.

@hsutter
Copy link
Owner

hsutter commented Dec 26, 2022

P.S.: There are other issues with default arguments, such as:

  • They are conceptually code that's injected into the call site. (That should raise a variety of yellow flags, if not necessarily red ones.)
  • They cause substitutability surprises for polymorphic function calls. Most famously the problems we have to teach about in today's C++ are with virtual functions, where overrides can have different default arguments but callers use the defaults for the function they're statically calling. But other types of polymorphism are dealt with differently in today's C++, the next biggest example being that we just ban default arguments for function specializations. So today's language is also inconsistent about polymorphism and default arguments... because for virtual functions we allow overrides to have default arguments (and have to teach people not to use different ones), whereas for function template specializations we don't allow specializations to have default arguments (we always use the ones in the base template), yet both are examples of polymorphism. Cpp2 currently intends to deal with them the same way: no default arguments.

Azmah-Bad pushed a commit to Azmah-Bad/cppfront that referenced this issue Feb 24, 2023
@JohelEGP
Copy link
Contributor

They are conceptually code that's injected into the call site. (That should raise a variety of yellow flags, if not necessarily red ones.)

  • Cpp2 currently intends to deal with them the same way: no default arguments.

What about std::source_location? As it stands, one can't author a Cpp2 function that makes use of it.

@hsutter
Copy link
Owner

hsutter commented Apr 20, 2023

std::source_location is a good counterexample.

BTW, preconditions are also conceptually code that's injected into the call site, so that isn't inherently problematic, it's just something to be cautious with (yellow flag vs red flag).

@JohelEGP
Copy link
Contributor

  • [...] So today's language is also inconsistent about polymorphism and default arguments... because for virtual functions we allow overrides to have default arguments (and have to teach people not to use different ones), whereas for function template specializations we don't allow specializations to have default arguments (we always use the ones in the base template), yet both are examples of polymorphism. Cpp2 currently intends to deal with them the same way: no default arguments.

Is this also a point against default arguments for types in template parameter lists?
That could block many useful applications of default policies.

@JohelEGP
Copy link
Contributor

Maybe the answer is "no".
Or the diagnostic needs to be updated.
See https://cpp2.godbolt.org/z/fqxbTGaM9:

t: @struct <T, U: type = i32> type = {
  a: T;
  b: U;
}
main: () = {
  _ = :t = (0);
}
main.cpp2(1,36): error: Cpp2 is currently exploring the path of not allowing default arguments - use overloading instead
main.cpp2(1,36): error: invalid template parameter list (at '=')

@hsutter
Copy link
Owner

hsutter commented Aug 13, 2023

Hmm, perhaps a semantic rule that the default initializer of a template argument is restricted to be an id-expression or something like that?

You could still create dependencies by referring to nested types but most cases would be fine and the ones that don't work just don't work (and could get better diagnostics in the future).

@JohelEGP
Copy link
Contributor

At #387 (comment), I suggest simplifying type-id by using id-expression.
So I think type-id is fits better.
Otherwise, * i32 wouldn't be a valid initializer for T in f: <T = * i32> () = { }.

But what about non-type template parameters?
I suppose an expression is OK for those.

@hsutter
Copy link
Owner

hsutter commented Aug 13, 2023

Good points.

For NTTPs, an expression has the same ordering problem as general value parameter default arguments.

For type parameters, I'll think about this some more, and maybe try out type-id. I'll treat is as lower priority though until people actually find use cases they can't live without... thanks again.

@JohelEGP
Copy link
Contributor

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

3 participants