Improving diagnostics with "Reasons:" and "Suggestions:"

[JohelEGP]

trafficstars

Title: Improving diagnostics with "Reasons:" and "Suggestions:".

Description:

Updating my code, I kept hitting: https://github.com/hsutter/cppfront/blob/68b716abe3e4b623e875d68a2b9b3f0efcd39b45/source/to_cpp1.h#L2433 I'm doubting the usefulness of this error as it is.

It's preventing me from writing

export from_window: (v) = v.from(window());

which copies v, a vector, into a point from window to v.

Following the comment a bit above: https://github.com/hsutter/cppfront/blob/68b716abe3e4b623e875d68a2b9b3f0efcd39b45/source/to_cpp1.h#L2411-L2412 I parenthesized the expression, which made it it OK.

Can we update these checks? copy seems to be as safe as any local variable (https://cpp2.godbolt.org/z/Y8nahPjhh works from Clang 12 and GCC 10 in C++20, to their HEAD versions in C++26).

Kind	Return	Action
in	param	Suggest in_ref.
in	param.X()	Suggest (expr) if param is not part of the return value, and in_ref param otherwise (maybe note that in can optimize to pass by copy).
copy	param	Allow.
copy	param.X()	Allow.
move	param	Suggest -> _, or -> cpp1_rvalue_ref<ParamType>, or allow -> forward_ref _ and also suggest it.
move	param.X()	Suggest (expr) if param is not part of the return value.

Sometimes, I'm left wondering the reason for some of the rejections. It'd be nice to centralize the documentation of how Cpp2 features obsolete guidance. That would make it easier to challenge some of decisions, and maybe even help notice further improvements.

For example, given the table above:

Kind	Return	Effect
in	param	Rejected because in can optimize to pass by copy. Use in_ref instead.
in	param.X()	Rejected because the identity of param might live in the resulting value of the expression.
copy	param	Allow for being as safe as using any local variable instead of param.
copy	param.X()	Allow for being as safe as using any local variable instead of param.
move	param	Rejected because the corresponding argument may be a prvalue.
move	param.X()	Rejected because the identity of param might live in the resulting value of the expression.

In fact, this reminds me of some talk(s) where the speaker(s) mention how the compilers of other programming languages have much nicer diagnostics. Something like how those compilers are a friend in the development journey rather than a tool.

Taking all of the above, consider the following Cpp2 code fragment and the contents of a desired diagnostic (https://cpp2.godbolt.org/z/n85qhcjbj).

  private validate: (this) -> bool = std::all_of(contents.begin(), contents.end(), :(x) = x.meets_the_requirements());

main.cpp2(4,91): error: a 'forward' return type cannot return an 'in' parameter
- Reason:
    `in param` can optimize to pass by copy (<https://hsutter.github.io/cppfront/cpp2/functions/#parameters>).
    In that case, returning `param` would return a [dangling reference](https://en.cppreference.com/w/cpp/language/reference#Dangling_references).
- Suggestion(s):
    - If the returned expression doesn't reference `param`,
      wrap it in parentheses to indicate so by convention.
    - If the returned expression references `param`,
      change its declaration to `in_ref param`.
- Note(s): `-> forward _` is implicit because the statement of the function is an expression.
           `:(   x   )              =          x.meets_the_requirements()`    expands to
           `:(in x: _) -> forward _ = { return x.meets_the_requirements(); }`
                       ~~~^~~~~~~~~

PS: I don't think this is a [SUGGESTION].