Quuxplusone added inline comments.
================
Comment at: clang/test/SemaCXX/deduced-return-type-cxx14.cpp:116
-auto &void_ret_2() {} // expected-error {{cannot deduce return type 'auto &'
for function with no return statements}}
+auto &void_ret_2() {} // expected-error {{cannot form a reference to 'void'}}
const auto void_ret_3() {} // ok, return type 'const void' is adjusted to
'void'
----------------
sammccall wrote:
> Quuxplusone wrote:
> > sammccall wrote:
> > > This feels like a regression, this diagnostic is just attached to the end
> > > of the function and there's no longer any explicit indication that the
> > > return type or deduction is involved.
> > >
> > > It may be worth keeping the explicit check in `ActOnFinishFunctionBody`
> > > before the deduction happens to improve the diagnostic in this case.
> > >
> > > (I don't think it's important to do this in the return-with-no-argument
> > > case, since the error will point at the return statement which provides
> > > enough context I think)
> > I agree about the (mild) regression but am not sure what to do about it. I
> > think it might help if Clang emitted a note `note: during return type
> > deduction here` (pointer to the offending `return` statement or
> > end-of-function); do you think I should do that? and if so, how?
> >
> > > It may be worth keeping the explicit check in `ActOnFinishFunctionBody`
> > > before the deduction happens to improve the diagnostic in this case.
> >
> > I can't picture what you mean; can you suggest a specific patch?
> >
> > In case it matters, I'm a little leery of hard-coding a special case like
> > we had in the old `DeduceFunctionTypeFromReturnExpr` lines 3814–3816,
> > ```
> > // Deduction here can only succeed if the return type is exactly 'cv
> > auto'
> > // or 'decltype(auto)', so just check for that case directly.
> > if (!OrigResultType.getType()->getAs<AutoType>()) {
> > ```
> > because special-casing `void` is kind of how we got into this mess in the
> > first place.
> > I can't picture what you mean; can you suggest a specific patch?
> > In case it matters, I'm a little leery of hard-coding a special case like
> > we had in the old
> > DeduceFunctionTypeFromReturnExpr
>
> Yeah, so that was pretty much my suggestion: keep the `getAs<AutoType>()`
> check in `ActOnFinishFunctionBody` to reject the most common cases with a
> good diagnostic, and then if it passes do the actual deduction.
> I agree this is ugly, I just didn't really have a better idea.
>
> > I think it might help if Clang emitted a note note: during return type
> > deduction here
>
> This is a nice idea, maybe better than the existing diagnostic.
>
> I haven't added notes myself, but I think all you have to do is `add a
> note_*` entry to DiagnosticSemaKinds.td, and emit it after the primary
> diagnostic as you'd emit any other diagnostic.
> Here if you see `DAR_Failed` then I think you know the call emitted a
> diagnostic and you can add the note. I'm not sure about
> `DAR_FailedAlreadyDiagnosed` - if it means no new diagnostic was emitted then
> you also don't want to add your note.
>
> (You'd want to do this in the `if (RetExpr)` case too of course)
>
> > pointer to the offending return statement or end-of-function
>
> The main diagnostics is going to point at that location already. So it might
> be more helpful to point the "during return type deduction" note at the
> return type written in the function declaration:
>
> ```
> return;
> ^~~~~~
> error: cannot form reference to 'void'
>
> auto& foo() {
> ^~~~
> note: deducing return type for 'foo'
> ```
@rsmith: This is the discussion that led to D119778 adding the note. I suppose
that by keeping all the special-case error messages in
`ActOnFinishFunctionBody`, we've reduced the benefit of the note.
In particular, `void_ret_2` no longer has this "regression," so there's no
longer any need for the note here — in fact, as you mentioned, the note is a
bit noisy.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D119184/new/
https://reviews.llvm.org/D119184
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
