diagnose auto deduction from void more specifically, fix for [GH #27]

[mohitmishra786]

Fixes #27
deducing auto from a void expression used to say only cannot deduce "auto" type, because void is incomplete and deduction stopped before it could name the type. Those cases now say cannot deduce "auto" type from type "void". That covers the reported example, auto x = g(), auto &&, const auto, auto *, new auto(...), and auto * return types. decltype(auto) is unchanged: deduction still succeeds, and the existing incomplete type "void" is not allowed diagnostic is issued.

one leftover generic message remains on auto *q = new auto(g()). The new auto part uses the new wording. The pointer variable then sees a failed new-expression, not a void expression, so it keeps the generic message.

testing is done like below
edg-docker-test --non-interactive --posture=quick-c-fe changes/GH27 on the linux-gcc-debug Docker build. The test is tests/tests/changes/GH27.sft.cpp (//type:fn, --c++23). The first run passed with no previous output. -W wrote tests/tests/changes/.GH27.rto/default.1.1.txt, and a second run matched that recording.

[mohitmishra786]

@wchilders-nvidia @daveedvdv could you please review this PR too?
thanks for your time ;-)

[nv-cmeerw]

I'd prefer auto and decltype(auto) to behave the same way here (and the deduction failure error seems somewhat misleading).

Maybe some (related) cases to think about (with the kind of error I'd expect here):

struct Incomplete &getIncomplete();

void f()
{
  auto v1 = void(); // error: incomplete type
  decltype(auto) v2 = void(); // error: incomplete type
  auto v3 = getIncomplete(); // error: incomplete type

  auto *v4 = void(); // error: cannot deduce
  auto *v5 = 1;   // error: cannot deduce
}

I wouldn't actually mind putting more information about the types for deduction failures in the diagnostics (for both v4 and v5 above)

[mohitmishra786]

I'd prefer auto and decltype(auto) to behave the same way here (and the deduction failure error seems somewhat misleading).

Maybe some (related) cases to think about (with the kind of error I'd expect here):

struct Incomplete &getIncomplete();

void f()
{
  auto v1 = void(); // error: incomplete type
  decltype(auto) v2 = void(); // error: incomplete type
  auto v3 = getIncomplete(); // error: incomplete type

  auto *v4 = void(); // error: cannot deduce
  auto *v5 = 1;   // error: cannot deduce
}

I wouldn't actually mind putting more information about the types for deduction failures in the diagnostics (for both v4 and v5 above)

indeed true, yeah, calling that a deduction failure was the wrong diagnosis. For auto v = void() the type really is void, and the problem is that a variable can't have it. decltype(auto) already said "incomplete type", so plain auto now does the same.

auto v1 = void(), decltype(auto) v2 = void(), and auto v3 = getIncomplete() all report an incomplete type. auto *v4 = void() and auto *v5 = 1 still fail deduction, with the existing "cannot deduce "auto" type" message. I didn't add the declared type and the initializer type onto every deduction failure. That string is recorded in a lot of tests, and the two cases above already match what you wrote. I can do the richer message separately if you still want it.

The CI mismatches were from the old wording. Recordings that now say incomplete type are updated, and the auto * return tests are back to "cannot deduce".

[mohitmishra786]

@wchilders-nvidia @daveedvdv @nv-cmeerw updated the PR as due to conflict of last changes, please check once more in you free time

[daveedvdv-nvidia]

Hi @mohitmishra786,

Thanks for working on this improvement!

Can you add changes for the output differences in existing tests. See e.g. https://github.com/edgcpp/compiler/actions/runs/37155984995/job/111792273841

[mohitmishra786]

Hi @mohitmishra786,

Thanks for working on this improvement!

Can you add changes for the output differences in existing tests. See e.g. https://github.com/edgcpp/compiler/actions/runs/37155984995/job/111792273841

That job is the earlier run, from before the recording updates. The tests whose output changed from "cannot deduce "auto" type" to the incomplete-type diagnostic now have updated default recordings: dcl.spec.auto/p2-1z, expr.prim.lambda/p11-1y, stmt.ranged/p1, auto-subst-failure, lambda-unevaluated, matrix-type-builtins-disabled, typeid-requires-typeinfo, and the new GH27 test. I rebased onto main again so the Changes file includes the 7.1 notes.

Maybe we can trigger fresh run of CI and see everything again.

[mohitmishra786]

Seems awaiting maintainers approval for the run

[nv-cmeerw]

This if should just be folded into the enclosing if. The comment mentions "auto", but the condition tests for template parameter type (should probably use is_auto_type then). But I think this should just test for allow_incomplete_arg (and not test for "auto" at all here), as we only set allow_incomplete_arg for the placeholder type deduction case.

I am actually wondering if that whole is_incomplete_type check should go away completely. There is a deeper issue here, consider:

struct Incomplete &getIncomplete();

template<typename T>
void g(T, int) // 1
{ }

template<typename T>
void g(T &, long) // 2
{ }

void f()
{
  g(getIncomplete(), 0);
}

Currently, we accept this, because we fail deduction for 1 and just call 2 instead. But we should succeed deduction for 1 and then diagnose the incomplete type error there.

We don't need to fix that with this change, but would be a good follow-up issue then.

[mohitmishra786]

This if should just be folded into the enclosing if. The comment mentions "auto", but the condition tests for template parameter type (should probably use is_auto_type then). But I think this should just test for allow_incomplete_arg (and not test for "auto" at all here), as we only set allow_incomplete_arg for the placeholder type deduction case.

I am actually wondering if that whole is_incomplete_type check should go away completely. There is a deeper issue here, consider:

struct Incomplete &getIncomplete();

template<typename T>
void g(T, int) // 1
{ }

template<typename T>
void g(T &, long) // 2
{ }

void f()
{
  g(getIncomplete(), 0);
}

Currently, we accept this, because we fail deduction for 1 and just call 2 instead. But we should succeed deduction for 1 and then diagnose the incomplete type error there.

We don't need to fix that with this change, but would be a good follow-up issue then.

Updated the comment to talk about placeholder type deduction, and folded the check so it only looks at allow_incomplete_arg. The Changes entry no longer quotes the old diagnostic, and the examples use "// Error:".

I left the template case (g(getIncomplete(), 0) succeeding deduction and then diagnosing the incomplete type) for a follow-up. changes/GH27 still matches.

[mohitmishra786]

@nv-cmeerw update the pr as per the comments, workflows are yet to run after one of the maintainers approval
please review and let me know if any more changes or its good now

[mohitmishra786]

@nv-cmeerw I see ci good now, once back please review, thanks

[nv-cmeerw]

The need for this change isn't entirely obvious. Was there any test case that would show undesirable results without this change? If so, it might be worth adding one example to your GH27.sft.cpp test to demonstrate the need for this change.

Thanks for your continued work on this issue.