[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 413089.
Quuxplusone added a comment.
Herald added a project: All.

Rebased after landing D119184 . I'm no longer 
necessarily trying to land this note (it's not directly relevant to my 
interests anymore), but I'd like to show its CI as green before I abandon it. 
@rsmith would you like me to do something other than abandon this?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119778/new/

https://reviews.llvm.org/D119778

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p2-1z.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
  clang/test/CXX/dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp
  clang/test/SemaCUDA/autoret-global.cu
  clang/test/SemaCXX/cxx2b-consteval-if.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/deduced-return-void.cpp
  clang/test/SemaCXX/typo-correction-crash.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -114,6 +114,7 @@
   }
   template void g5() {
 ([]() -> C auto{ // expected-error-re {{deduced type {{.*}} does not satisfy}}
+// expected-note@-1 {{deducing return type for 'operator()'}}
  return T();
  }(), ...);
   }
@@ -174,26 +175,34 @@
   template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
 
   C auto f1() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+// expected-note@-1 {{deducing return type for 'f1'}}
 return void();
   }
   C auto f2() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+// expected-note@-1 {{deducing return type for 'f2'}}
 return;
   }
   C auto f3() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+// expected-note@-1 {{deducing return type for 'f3'}}
   }
   C decltype(auto) f4() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  // expected-note@-1 {{deducing return type for 'f4'}}
 return void();
   }
   C decltype(auto) f5() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  // expected-note@-1 {{deducing return type for 'f5'}}
 return;
   }
   C decltype(auto) f6() { // expected-error {{deduced type 'void' does not satisfy 'C'}}
+  // expected-note@-1 {{deducing return type for 'f6'}}
   }
   C auto& f7() { // expected-error {{cannot form a reference to 'void'}}
+ // expected-note@-1 {{deducing return type for 'f7'}}
 return void();
   }
   C auto& f8() {
 return; // expected-error {{cannot deduce return type 'C auto &' from omitted return expression}}
+// expected-note@-2 {{deducing return type for 'f8'}}
   }
   C auto& f9() { // expected-error {{cannot deduce return type 'C auto &' for function with no return statements}}
   }
Index: clang/test/SemaCXX/typo-correction-crash.cpp
===
--- clang/test/SemaCXX/typo-correction-crash.cpp
+++ clang/test/SemaCXX/typo-correction-crash.cpp
@@ -9,6 +9,7 @@
   return "s";
   return tes; // expected-error {{use of undeclared identifier 'tes'; did you mean 'test'?}}
   // expected-error@-1 {{deduced as 'int' here but deduced as 'const char *' in earlier}}
+  // expected-note@-4 {{deducing return type for 'check2'}}
 }
 
 template  struct is_same { static constexpr bool value = false; };
Index: clang/test/SemaCXX/deduced-return-void.cpp
===
--- clang/test/SemaCXX/deduced-return-void.cpp
+++ clang/test/SemaCXX/deduced-return-void.cpp
@@ -16,10 +16,12 @@
 auto f4() {
   return i;
   return; // expected-error {{'auto' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+  // expected-note@-3 {{deducing return type for 'f4'}}
 }
 auto f5() {
   return i;
   return void(); // expected-error {{'auto' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+ // expected-note@-3 {{deducing return type for 'f5'}}
 }
 
 auto l1 = []() { };
@@ -44,10 +46,12 @@
 decltype(auto) f4() {
   return i;
   return; // expected-error {{'decltype(auto)' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+  // expected-note@-3 {{deducing return type for 'f4'}}
 }
 decltype(auto) f5() {
   return i;
   return void(); // expected-error {{'decltype(auto)' in return type deduced as 'void' here but 

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D119778#3327161 , @Quuxplusone 
wrote:

> Apply @rsmith's suggested approach.

Thanks, the structure of the change looks good to me.

Looking through the test changes, I'm a bit torn by the value proposition of 
this note. Some of the errors were already pointing at the return type itself, 
and others were pointing at the return statement. I can see value in making 
sure that we mention both when diagnosing return type deduction errors, but 
pointing out the return type twice and never mentioning the return statement 
seems a bit noisy. I don't think I have any great suggestions here, though; 
there's not really a great way we can determine which of the two a diagnostic 
is pointing at so we can point the note at the other one.

The cases that I found in the tests that seem particularly bad today:

  template concept Never = false;
  void g();
  // error: cannot form a reference to 'void'
  auto () {
  // ...
  return g();
  }
  // error: deduced type 'void' does not satisfy 'Never'
  Never auto h() {
  // ...
  return g();
  }

... would both benefit from a note saying "return type deduced from returned 
expression of type 'void' here" or something like that, and aren't really 
helped much by this change. But I think noting *both* the return statement and 
the return type would be too much (then we guarantee we always have at least 
one noisy note). :(

What do you think? This seems on balance to be near the edge between a useful 
note and a noisy note, though in many of the specific cases it's pretty clearly 
on one side or the other.

>> I don't see any obvious way that this patch could be responsible for that 
>> failure, unless it's something like a pre-existing use of uninitialized 
>> memory or a use-after-free and this patch is just changing the happenstance 
>> behavior.
>
> Well, it might more likely be the fault of D119184 
>  rather than this one per se. D119184 
>  is doing things with `Context.VoidTy` that 
> weren't there before; could any of that new code be the culprit?

That seems more likely, but nothing in that patch is jumping out at me. 
Especially given that it only affects the behavior of return type deduction in 
a function with either no return statements or only `return;`, it's pretty 
weird that libc++ would even exercise the patch.




Comment at: clang/test/SemaCXX/deduced-return-void.cpp:153-154
 };
 auto l5 = []() -> auto& { // expected-error {{cannot form a reference to 
'void'}}
+  // expected-note@-1 {{deducing return type for 
'operator()'}}
   return i;

This seems like an unfortunate case: both the error and the note point at the 
return type and neither points at the return statement.



Comment at: clang/test/SemaTemplate/concepts.cpp:177-178
 
   C auto f1() { return void(); }   // expected-error {{deduced type 
'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing 
return type for 'f1'}}
   C auto f2() { return; }  // expected-error {{deduced type 
'void' does not satisfy 'C'}}

This is an unfortunate case: the existing error message already points at the 
`C auto` constraint, so we now have an error and a note both pointing at the 
return type, and nothing pointing to the `return` statement itself.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119778/new/

https://reviews.llvm.org/D119778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 409359.
Quuxplusone added a comment.

Apply @rsmith's suggested approach.

> I don't see any obvious way that this patch could be responsible for that 
> failure, unless it's something like a pre-existing use of uninitialized 
> memory or a use-after-free and this patch is just changing the happenstance 
> behavior.

Well, it might more likely be the fault of D119184 
 rather than this one per se. D119184 
 is doing things with `Context.VoidTy` that 
weren't there before; could any of that new code be the culprit?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119778/new/

https://reviews.llvm.org/D119778

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Sema/SemaStmt.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p2-1z.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
  clang/test/CXX/dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp
  clang/test/SemaCUDA/autoret-global.cu
  clang/test/SemaCXX/cxx2b-consteval-if.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/deduced-return-void.cpp
  clang/test/SemaCXX/typo-correction-crash.cpp
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -114,6 +114,7 @@
   }
   template void g5() {
 ([]() -> C auto{ // expected-error-re {{deduced type {{.*}} does not satisfy}}
+// expected-note@-1 {{deducing return type for 'operator()'}}
  return T();
  }(), ...);
   }
@@ -174,11 +175,17 @@
   template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
 
   C auto f1() { return void(); }   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f1'}}
   C auto f2() { return; }  // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f2'}}
   C auto f3() {}   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f3'}}
   C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f4'}}
   C decltype(auto) f5() { return; }// expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f5'}}
   C decltype(auto) f6() {} // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f6'}}
 
   void g() {
 f1();
Index: clang/test/SemaCXX/typo-correction-crash.cpp
===
--- clang/test/SemaCXX/typo-correction-crash.cpp
+++ clang/test/SemaCXX/typo-correction-crash.cpp
@@ -9,6 +9,7 @@
   return "s";
   return tes; // expected-error {{use of undeclared identifier 'tes'; did you mean 'test'?}}
   // expected-error@-1 {{deduced as 'int' here but deduced as 'const char *' in earlier}}
+  // expected-note@-4 {{deducing return type for 'check2'}}
 }
 
 template  struct is_same { static constexpr bool value = false; };
Index: clang/test/SemaCXX/deduced-return-void.cpp
===
--- clang/test/SemaCXX/deduced-return-void.cpp
+++ clang/test/SemaCXX/deduced-return-void.cpp
@@ -16,10 +16,12 @@
 auto f4() {
   return i;
   return; // expected-error {{'auto' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+  // expected-note@-3 {{deducing return type for 'f4'}}
 }
 auto f5() {
   return i;
   return void(); // expected-error {{'auto' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+ // expected-note@-3 {{deducing return type for 'f5'}}
 }
 
 auto l1 = []() { };
@@ -44,10 +46,12 @@
 decltype(auto) f4() {
   return i;
   return; // expected-error {{'decltype(auto)' in return type deduced as 'void' here but deduced as 'int' in earlier return statement}}
+  // expected-note@-3 {{deducing return type for 'f4'}}
 }
 decltype(auto) f5() {
   return i;
   return void(); // expected-error {{'decltype(auto)' in return type deduced as 'void' here but deduced as 'int' in earlier 

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added a comment.

In D119778#3326285 , @Quuxplusone 
wrote:

> I'll update with the `CodeSynthesisContext` approach in a little bit. But 
> meanwhile there's a problem with both the current patch //and// (I think even 
> more) with the new approach. @rsmith any help?:
> When I try my latest patch with the libc++ tests, with `bin/llvm-lit -sv 
> --param enable_modules=True` (which corresponds to the `-fmodules` compiler 
> flag), I get lots of errors like the following. Do you have any idea what's 
> going wrong?

I don't see any obvious way that this patch could be responsible for that 
failure, unless it's something like a pre-existing use of uninitialized memory 
or a use-after-free and this patch is just changing the happenstance behavior.

> I can't minimize the example beyond "all of libc++" because Modules prevents 
> `creduce` from doing anything meaningful. And unfortunately whatever's going 
> wrong seems to be //specifically// about Modules.

It'd likely be useful to know what the `Replacement`  type is, if you can 
observe this crash in a debugger. I think it's possible for a type that was 
canonical prior to serialization and deserialization to be non-canonical 
afterwards (perhaps because we picked a different declaration of the type to be 
the first one or merged types in a different order or something like that). It 
may be that we just need to map to a canonical type in some suitable place.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119778/new/

https://reviews.llvm.org/D119778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

I'll update with the `CodeSynthesisContext` approach in a little bit. But 
meanwhile there's a problem with both the current patch //and// (I think even 
more) with the new approach. @rsmith any help?:
When I try my latest patch with the libc++ tests, with `bin/llvm-lit -sv 
--param enable_modules=True` (which corresponds to the `-fmodules` compiler 
flag), I get lots of errors like the following. Do you have any idea what's 
going wrong?

I can't minimize the example beyond "all of libc++" because Modules prevents 
`creduce` from doing anything meaningful. And unfortunately whatever's going 
wrong seems to be //specifically// about Modules.

  Assertion failed: (Replacement.isCanonical() && "replacement types must 
always be canonical"), function getSubstTemplateTypeParmType, file 
/Users/aodwyer/llvm-project/clang/lib/AST/ASTContext.cpp, line 4691.
  PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ 
and include the crash backtrace, preprocessed source, and associated run script.
  Stack dump:
  0.Program arguments: /Users/aodwyer/llvm-project/build3/bin/clang-14 -cc1 
-triple x86_64-apple-macosx10.15.0 -Wundef-prefix=TARGET_OS_ 
-Werror=undef-prefix -Wdeprecated-objc-isa-usage 
-Werror=deprecated-objc-isa-usage -emit-obj -mrelax-all --mrelax-relocations 
-disable-free -clear-ast-before-backend -main-file-name rbegin.pass.cpp 
-mrelocation-model pic -pic-level 2 -mframe-pointer=all -ffp-contract=on 
-fno-rounding-math -funwind-tables=2 -target-sdk-version=10.15.6 
-fcompatibility-qualified-id-block-type-checking 
-fvisibility-inlines-hidden-static-local-var -target-cpu penryn -tune-cpu 
generic -mllvm -treat-scalable-fixed-error-as-warning -debugger-tuning=lldb 
-target-linker-version 609.8 -v 
-fcoverage-compilation-dir=/Users/aodwyer/llvm-project/build3/projects/libcxx/test/std/ranges/range.access
 -nostdinc++ -resource-dir /Users/aodwyer/llvm-project/build3/lib/clang/15.0.0 
-isysroot /Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk -isystem 
/Users/aodwyer/llvm-project/build3/include/c++/v1 -I 
/Users/aodwyer/llvm-project/build3/projects/libcxx/include/c++build -I 
/Users/aodwyer/llvm-project/libcxx/test/support -D _LIBCPP_DISABLE_AVAILABILITY 
-D _LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER -stdlib=libc++ -internal-isystem 
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/local/include 
-internal-isystem /Users/aodwyer/llvm-project/build3/lib/clang/15.0.0/include 
-internal-externc-isystem 
/Library/Developer/CommandLineTools/SDKs/MacOSX10.15.sdk/usr/include -Werror 
-Wall -Wextra -Wshadow -Wundef -Wno-unused-command-line-argument 
-Wno-attributes -Wno-pessimizing-move -Wno-c++11-extensions 
-Wno-user-defined-literals -Wno-noexcept-type -Wno-atomic-alignment 
-Wsign-compare -Wunused-variable -Wunused-parameter -Wunreachable-code 
-Wno-unused-local-typedef -Werror=thread-safety -Wuser-defined-warnings 
-std=c++20 -fdeprecated-macro 
-fdebug-compilation-dir=/Users/aodwyer/llvm-project/build3/projects/libcxx/test/std/ranges/range.access
 -ferror-limit 19 -stack-protector 1 -fblocks -fencode-extended-block-signature 
-fcoroutines-ts -fregister-global-dtors-with-atexit -fgnuc-version=4.2.1 
-fmodules -fimplicit-module-maps 
-fmodules-cache-path=/var/folders/0l/9t0yv2890_g4wgmy53n_mg7wgy/C/clang/ModuleCache
 -fmodules-validate-system-headers -fcxx-exceptions -fexceptions 
-fmax-type-align=16 -D__GCC_HAVE_DWARF2_CFI_ASM=1 -o 
/var/folders/0l/9t0yv2890_g4wgmy53n_mg7wgy/T/lit-tmp-fkh9o_f4/rbegin-d58043.o
 -x c++ 
/Users/aodwyer/llvm-project/libcxx/test/std/ranges/range.access/rbegin.pass.cpp
  1.
/Users/aodwyer/llvm-project/libcxx/test/support/test_iterators.h:524:30: 
current parser token '{'
  Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH 
or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
  0  clang-14 0x00010634385d 
llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 61
  1  clang-14 0x000106343ddb 
PrintStackTraceSignalHandler(void*) + 27
  2  clang-14 0x000106341cfa llvm::sys::RunSignalHandlers() 
+ 138
  3  clang-14 0x0001063457af SignalHandler(int) + 223
  4  libsystem_platform.dylib 0x7fff677255fd _sigtramp + 29
  5  libsystem_platform.dylib 0x7ffeed912f58 _sigtramp + 
18446744071664753016
  6  libsystem_c.dylib0x7fff675fb808 abort + 120
  7  libsystem_c.dylib0x7fff675faac6 err + 0
  8  clang-14 0x00010b769ce1 
clang::ASTContext::getSubstTemplateTypeParmType(clang::TemplateTypeParmType 
const*, clang::QualType) const + 161
  9  clang-14 0x00010b1feb40 (anonymous 
namespace)::TemplateInstantiator::TransformTemplateTypeParmType(clang::TypeLocBuilder&,
 clang::TemplateTypeParmTypeLoc) + 1456
  10 clang-14 0x00010b1ba74f 
clang::TreeTransform<(anonymous 

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3804-3809
+if (DAR != DAR_Succeeded) {
+  if (OrigResultType.getBeginLoc().isValid())
+Diag(OrigResultType.getBeginLoc(), diag::note_deducing_return_type_for)
+<< FD << OrigResultType.getSourceRange();
   return true;
+}

The approach of tacking a note onto the end of someone else's diagnostic is an 
antipattern and should be avoided in new code. For example, if `DeduceAutoType` 
produces an error (which is shown) then a warning (which is hidden), the note 
will be attached to the warning, and will not appear.

The right thing to do is to create a new kind of `CodeSynthesisContext` for 
this case and push that. Then we'll use the same logic as in template 
instantiation backtraces, and the note will get properly attached to the first 
emitted diagnostic within the context. This will also remove the need to track 
down all the different places that emit diagnostics (like we see below): you 
just push the context when you start doing the action you want to attach the 
note to and pop it when you're done. It'll also properly order the note with 
respect to other context notes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119778/new/

https://reviews.llvm.org/D119778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.



Comment at: clang/lib/Sema/SemaStmt.cpp:3805
+if (DAR != DAR_Succeeded) {
+  if (OrigResultType.getBeginLoc().isValid())
+Diag(OrigResultType.getBeginLoc(), diag::note_deducing_return_type_for)

> I am curious about the behavior if we removed the guard `if 
> (OrigResultType.getBeginLoc().isValid())`.

That caused the note to be emitted in some cases without a source location 
(similar to https://github.com/llvm/llvm-project/issues/29054 ). I admit I 
should check to make sure we have a test case for each of these four `if`s, 
though; I suspect we don't.
```
$ cat x.cpp
auto f = []() { return x; };

$ clang++ x.cpp
x.cpp:1:24: error: use of undeclared identifier 'x'
auto f = []() { return x; };
   ^
note: deducing return type for 'operator()'
1 error generated.
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119778/new/

https://reviews.llvm.org/D119778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone updated this revision to Diff 409095.
Quuxplusone added a comment.

Rebase; adjust more expected output in tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119778/new/

https://reviews.llvm.org/D119778

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaStmt.cpp
  clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p2-1z.cpp
  clang/test/CXX/dcl/dcl.spec/dcl.type/dcl.spec.auto/p6.cpp
  clang/test/CXX/temp/temp.constr/temp.constr.constr/non-function-templates.cpp
  clang/test/Sema/invalid-bitwidth-expr.mm
  clang/test/SemaCXX/cxx20-p0388-unbound-ary.cpp
  clang/test/SemaCXX/cxx2b-consteval-if.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/deduced-return-void.cpp
  clang/test/SemaCXX/std-compare-cxx2a.cpp
  clang/test/SemaCXX/typo-correction-crash.cpp
  clang/test/SemaOpenCLCXX/template-astype.cl
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -114,6 +114,7 @@
   }
   template void g5() {
 ([]() -> C auto{ // expected-error-re {{deduced type {{.*}} does not satisfy}}
+// expected-note@-1 {{deducing return type for 'operator()'}}
  return T();
  }(), ...);
   }
@@ -174,11 +175,17 @@
   template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
 
   C auto f1() { return void(); }   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f1'}}
   C auto f2() { return; }  // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f2'}}
   C auto f3() {}   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f3'}}
   C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f4'}}
   C decltype(auto) f5() { return; }// expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f5'}}
   C decltype(auto) f6() {} // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f6'}}
 
   void g() {
 f1();
Index: clang/test/SemaOpenCLCXX/template-astype.cl
===
--- clang/test/SemaOpenCLCXX/template-astype.cl
+++ clang/test/SemaOpenCLCXX/template-astype.cl
@@ -11,6 +11,7 @@
 
 auto neg_test_int(int x) { return templated_astype(x); }
 // expected-note@-1{{in instantiation of function template specialization 'templated_astype' requested here}}
+// expected-note@-2{{deducing return type for 'neg_test_int'}}
 
 auto test_short4(short4 x) { return templated_astype(x); }
 
Index: clang/test/SemaCXX/typo-correction-crash.cpp
===
--- clang/test/SemaCXX/typo-correction-crash.cpp
+++ clang/test/SemaCXX/typo-correction-crash.cpp
@@ -2,6 +2,7 @@
 auto check1() {
   return 1;
   return s; // expected-error {{use of undeclared identifier 's'}}
+// expected-note@-3 {{deducing return type for 'check1'}}
 }
 
 int test = 11; // expected-note 2 {{'test' declared here}}
@@ -9,6 +10,7 @@
   return "s";
   return tes; // expected-error {{use of undeclared identifier 'tes'; did you mean 'test'?}}
   // expected-error@-1 {{deduced as 'int' here but deduced as 'const char *' in earlier}}
+  // expected-note@-4 {{deducing return type for 'check2'}}
 }
 
 template  struct is_same { static constexpr bool value = false; };
Index: clang/test/SemaCXX/std-compare-cxx2a.cpp
===
--- clang/test/SemaCXX/std-compare-cxx2a.cpp
+++ clang/test/SemaCXX/std-compare-cxx2a.cpp
@@ -32,6 +32,7 @@
 auto compare_incomplete_test() {
   // expected-error@+1 {{incomplete type 'std::partial_ordering' where a complete type is required}}
   return (-1.2 <=> 123.0);
+  // expected-note@-3 {{deducing return type for 'compare_incomplete_test'}}
 }
 
 namespace std {
@@ -45,6 +46,7 @@
 auto missing_member_test() {
   // expected-error@+1 {{standard library implementation of 'std::partial_ordering' is not supported; member 'equivalent' is missing}}
   return (1.0 <=> 1.0);
+  // expected-note@-3 {{deducing return type for 'missing_member_test'}}
 }
 
 namespace std {
@@ -59,6 +61,7 @@
 auto 

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added a comment.

I am curious about the behavior if we removed the guard `if 
(OrigResultType.getBeginLoc().isValid())`.




Comment at: clang/lib/Sema/SemaStmt.cpp:3804-3807
+if (DAR != DAR_Succeeded) {
+  if (OrigResultType.getBeginLoc().isValid())
+Diag(OrigResultType.getBeginLoc(), diag::note_deducing_return_type_for)
+<< FD << OrigResultType.getSourceRange();

What if merge this one into above block where the diagnostic happens.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D119778/new/

https://reviews.llvm.org/D119778

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119778: [clang] Add a note "deducing return type for 'foo'"

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone created this revision.
Quuxplusone added reviewers: sammccall, rsmith, dblaikie, majnemer, mizvekov, 
ChuanqiXu.
Quuxplusone added a project: clang.
Quuxplusone requested review of this revision.
Herald added a subscriber: cfe-commits.

Emit the note when we fail to deduce a return type, or deduce conflicting 
return types.

It's emitted a //little// too eagerly, e.g. I wish we could avoid it in cases 
like

  $ bin/clang++ x.cpp
  x.cpp:1:19: error: use of undeclared identifier 'x'
  auto f() { return x; }
^
  x.cpp:1:1: note: deducing return type for 'f'
  auto f() { return x; }
  ^~~~
  1 error generated.

But Clang doesn't seem to be able to distinguish "parsing and type-checking the 
return //expression//" from "deducing the function's actual return //type//," 
and I'm not inclined to dig this rabbit hole deeper than I already have. I 
think this is still a reasonably clean place to stop digging.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D119778

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaStmt.cpp
  clang/test/Sema/invalid-bitwidth-expr.mm
  clang/test/SemaCXX/cxx20-p0388-unbound-ary.cpp
  clang/test/SemaCXX/cxx2b-consteval-if.cpp
  clang/test/SemaCXX/deduced-return-type-cxx14.cpp
  clang/test/SemaCXX/deduced-return-void.cpp
  clang/test/SemaCXX/std-compare-cxx2a.cpp
  clang/test/SemaCXX/typo-correction-crash.cpp
  clang/test/SemaOpenCLCXX/template-astype.cl
  clang/test/SemaTemplate/concepts.cpp

Index: clang/test/SemaTemplate/concepts.cpp
===
--- clang/test/SemaTemplate/concepts.cpp
+++ clang/test/SemaTemplate/concepts.cpp
@@ -114,6 +114,7 @@
   }
   template void g5() {
 ([]() -> C auto{ // expected-error-re {{deduced type {{.*}} does not satisfy}}
+// expected-note@-1 {{deducing return type for 'operator()'}}
  return T();
  }(), ...);
   }
@@ -174,11 +175,17 @@
   template concept C = false; // expected-note 6 {{because 'false' evaluated to false}}
 
   C auto f1() { return void(); }   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f1'}}
   C auto f2() { return; }  // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f2'}}
   C auto f3() {}   // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f3'}}
   C decltype(auto) f4() { return void(); } // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f4'}}
   C decltype(auto) f5() { return; }// expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f5'}}
   C decltype(auto) f6() {} // expected-error {{deduced type 'void' does not satisfy 'C'}}
+   // expected-note@-1 {{deducing return type for 'f6'}}
 
   void g() {
 f1();
Index: clang/test/SemaOpenCLCXX/template-astype.cl
===
--- clang/test/SemaOpenCLCXX/template-astype.cl
+++ clang/test/SemaOpenCLCXX/template-astype.cl
@@ -11,6 +11,7 @@
 
 auto neg_test_int(int x) { return templated_astype(x); }
 // expected-note@-1{{in instantiation of function template specialization 'templated_astype' requested here}}
+// expected-note@-2{{deducing return type for 'neg_test_int'}}
 
 auto test_short4(short4 x) { return templated_astype(x); }
 
Index: clang/test/SemaCXX/typo-correction-crash.cpp
===
--- clang/test/SemaCXX/typo-correction-crash.cpp
+++ clang/test/SemaCXX/typo-correction-crash.cpp
@@ -2,6 +2,7 @@
 auto check1() {
   return 1;
   return s; // expected-error {{use of undeclared identifier 's'}}
+// expected-note@-3 {{deducing return type for 'check1'}}
 }
 
 int test = 11; // expected-note 2 {{'test' declared here}}
@@ -9,6 +10,7 @@
   return "s";
   return tes; // expected-error {{use of undeclared identifier 'tes'; did you mean 'test'?}}
   // expected-error@-1 {{deduced as 'int' here but deduced as 'const char *' in earlier}}
+  // expected-note@-4 {{deducing return type for 'check2'}}
 }
 
 template  struct is_same { static constexpr bool value = false; };
Index: clang/test/SemaCXX/std-compare-cxx2a.cpp
===
--- clang/test/SemaCXX/std-compare-cxx2a.cpp
+++ clang/test/SemaCXX/std-compare-cxx2a.cpp
@@ -32,6 +32,7 @@
 auto compare_incomplete_test() {
   //