[clang] More informative mixed namespace diagnostics

[urnathan]

Summary

Continuing the improvement of std/std::experimental mixing diagnostics ...

First, let's check we get a TemplateDecl, before complaining about where it might have been found.

Second, if it came from an unexpected place, show where that location is with a 'declared here' note.

(Still an error when std and std::experimental both have decls)

[• Quuxplusone]

clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp

3

Instead of doing this -verify=expected,declared, I have another trick for you from the libcxx test suite. :) IIUC, the compiler output we expect is

../clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp:53:7: warning: support for std::experimental::coroutine_traits will be removed in LLVM 15; use std::coroutine_traits instead [-Wdeprecated-experimental-coroutine]
  for co_await (auto i : arr) {} // expected-warning {{support for std::experimental::coroutine_traits will be removed}}
      ^
../clang/test/SemaCXX/Inputs/std-coroutine-exp-namespace.h:8:8: note: 'coroutine_traits' declared here
struct coroutine_traits { using promise_type = typename Ret::promise_type; };
       ^

So, down on line 53, I think we should write

MyForLoopArrayAwaiter g() {
  int arr[10] = {0};
  for co_await (auto i : arr) {}
  // expected-warning@-1 {{support for std::experimental::coroutine_traits will be removed}}
  // expected-note@Inputs/std-coroutine-exp-namespace.h:8 {{'coroutine_traits' declared here}}
  // expected-error@-3 {{call to deleted member function 'await_transform'}}
  // expected-note@-4 {{'await_transform' implicitly required by 'co_await' here}}
}

And then on line 3 we can just write -verify. And in Inputs/std-coroutine-exp-namespace.h itself, we don't need to write anything.
This keeps the test more "all in one place," which I think is more readable/understandable.

(Likewise throughout. I assume this will have a big effect on all these tests.)

[urnathan]

Update expected-diag markers

[ChuanqiXu]

LGTM

clang/lib/Sema/SemaCoroutine.cpp

if (!StdCoroutineTraitsCache) {
 
     if (!InStd && !InExp) {
       // The goggles, they found nothing!
       Diag(KwLoc, diag::err_implied_coroutine_type_not_found)
           << "std::coroutine_traits";
       return nullptr;
     }
 
-    if (!InStd) {
-      // Found only in std::experimental.
-      Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
-          << "coroutine_traits";
-    } else if (InExp) {
-      // Found in std and std::experimental.
-      Diag(KwLoc,
-           diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
-      Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
-          << "coroutine_traits";
-      return nullptr;
-    }
-
     // Prefer ::std to std::experimental.
     auto &Result = InStd ? ResStd : ResExp;
     CoroTraitsNamespaceCache = InStd ? StdSpace : ExpSpace;
 
     // coroutine_traits is required to be a class template.
-    if (!(StdCoroutineTraitsCache = Result.getAsSingle<ClassTemplateDecl>())) {
+    StdCoroutineTraitsCache = Result.getAsSingle<ClassTemplateDecl>();
+    if (!StdCoroutineTraitsCache) {
       Result.suppressDiagnostics();
       NamedDecl *Found = *Result.begin();
       Diag(Found->getLocation(), diag::err_malformed_std_coroutine_traits);
       return nullptr;
     }
+
+    if (InExp) {
+      // Found in std::experimental
+      Diag(KwLoc, diag::warn_deprecated_coroutine_namespace)
+          << "coroutine_traits";
+      ResExp.suppressDiagnostics();
+      auto *Found = *ResExp.begin();
+      Diag(Found->getLocation(), diag::note_entity_declared_at) << Found;
+
+      if (InStd) {
+        // Also found in std
+        Diag(KwLoc,
+             diag::err_mixed_use_std_and_experimental_namespace_for_coroutine);
+        Diag(StdCoroutineTraitsCache->getLocation(),
+             diag::note_entity_declared_at)
+            << StdCoroutineTraitsCache;
+
+        return nullptr;
+      }
+    }
   }
   Namespace = CoroTraitsNamespaceCache;
   return StdCoroutineTraitsCache;
 }

clang/test/SemaCXX/Inputs/std-coroutine-exp-namespace.h

-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify -fblocks -Wno-unreachable-code -Wno-unused-value
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify=expected,declared -fblocks -Wno-unreachable-code -Wno-unused-value
 #ifndef STD_COROUTINE_EXPERIMENTAL_H
 #define STD_COROUTINE_EXPERIMENTAL_H
 
 namespace std {
 namespace experimental {
 template <class Ret, typename... T>
 struct coroutine_traits { using promise_type = typename Ret::promise_type; };
+// declared-note@-1{{declared here}}
 
 template <class Promise = void>
 struct coroutine_handle {
   static coroutine_handle from_address(void *) noexcept;
 };
 template <>
 struct coroutine_handle<void> {
   template <class PromiseType>

clang/test/SemaCXX/Inputs/std-coroutine.h

 // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++20 -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify -fblocks -Wno-unreachable-code -Wno-unused-value
 #ifndef STD_COROUTINE_H
 #define STD_COROUTINE_H
 
 namespace std {
 
 template <class Ret, typename... T>
 struct coroutine_traits { using promise_type = typename Ret::promise_type; };
+// declared-note@-1{{declared here}}
 
 template <class Promise = void>
 struct coroutine_handle {
   static coroutine_handle from_address(void *) noexcept;
 };
 template <>
 struct coroutine_handle<void> {
   template <class PromiseType>

clang/test/SemaCXX/co_await-range-for-exp-namespace.cpp

 // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts \
-// RUN:    -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify \
-// RUN:    -fblocks
+// RUN:    -fsyntax-only -Wignored-qualifiers -Wno-error=return-type \
+// RUN:    -verify=expected,declared -fblocks
 #include "Inputs/std-coroutine-exp-namespace.h"
 
 using namespace std::experimental;
 
 template <class Begin>
 struct Awaiter {
   bool await_ready();
   void await_suspend(coroutine_handle<>);

clang/test/SemaCXX/coreturn-eh-exp-namespace.cpp

-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fcxx-exceptions -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify -fblocks -Wall -Wextra -Wno-error=unreachable-code
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fcxx-exceptions -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify=expected,declared -fblocks -Wall -Wextra -Wno-error=unreachable-code
 
 #include "Inputs/std-coroutine-exp-namespace.h"
 
 using std::experimental::suspend_always;
 using std::experimental::suspend_never;
 
 struct awaitable {
   bool await_ready();

clang/test/SemaCXX/coreturn-exp-namespace.cpp

-// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify -fblocks -Wall -Wextra -Wno-error=unreachable-code
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify=expected,declared -fblocks -Wall -Wextra -Wno-error=unreachable-code
 #include "Inputs/std-coroutine-exp-namespace.h"
 
 using std::experimental::suspend_always;
 using std::experimental::suspend_never;
 
 struct awaitable {
   bool await_ready();
   void await_suspend(std::experimental::coroutine_handle<>); // FIXME: coroutine_handle

clang/test/SemaCXX/coroutine-final-suspend-noexcept-exp-namespace.cpp

 // This file contains references to sections of the Coroutines TS, which can be
 // found at http://wg21.link/coroutines.
 
 // RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -Wunused-result
 
 namespace std {
 namespace experimental {
 template <class Ret, typename... T>
 struct coroutine_traits { using promise_type = typename Ret::promise_type; };
+// expected-note@-1{{declared here}}
 
 template <class Promise = void>
 struct coroutine_handle {
   static coroutine_handle from_address(void *); // expected-note 2 {{must be declared with 'noexcept'}}
 };
 template <>
 struct coroutine_handle<void> {
   template <class PromiseType>

clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp

-// This file is to test the mixed use of `std::experimental::coroutine*` and `std::coroutine*`
-// wouldn't make the compiler to crash and emit the diagnostic message correctly.
-// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+// This file is to test the mixed use of `std::experimental::coroutine*` and
+// `std::coroutine*` wouldn't make the compiler to crash and emit the diagnostic
+// message correctly.
+
+// RUN: %clang_cc1 -verify=expected,declared -std=c++20 -fsyntax-only %s
 
 #include "Inputs/std-coroutine-exp-namespace.h"
-#include "Inputs/std-coroutine.h"
+#include "Inputs/std-coroutine.h" // Second
 
 struct my_awaitable {
   bool await_ready() noexcept;
   void await_suspend(std::experimental::coroutine_handle<> coro) noexcept;
   void await_resume() noexcept;
 };
 
 struct promise_void {

clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp

-// This file is to test the mixed use of `std::experimental::coroutine_traits` and `std::coroutine_traits`
-// which is similar to coroutine-mixed-exp-namesapce. This file tests the relative order of
-// included header wouldn't affect the diagnostic messages.
-// RUN: %clang_cc1 -verify -std=c++20 -fsyntax-only %s
+// This file is to test the mixed use of `std::experimental::coroutine_traits`
+// and `std::coroutine_traits` which is similar to
+// coroutine-mixed-exp-namespace. This file tests the relative order of included
+// header wouldn't affect the diagnostic messages.
 
+// RUN: %clang_cc1 -verify=expected,declared -std=c++20 -fsyntax-only %s
+
+#include "Inputs/std-coroutine.h" // First
 #include "Inputs/std-coroutine-exp-namespace.h"
-#include "Inputs/std-coroutine.h"
 
 struct my_awaitable {
   bool await_ready() noexcept;
   void await_suspend(std::experimental::coroutine_handle<> coro) noexcept;
   void await_resume() noexcept;
 };
 
 struct promise_void {

clang/test/SemaCXX/coroutine-rvo-exp-namespace.cpp

 
 template <class T>
 struct traits_sfinae_base<T, void_t<typename T::promise_type>> {
   using promise_type = typename T::promise_type;
 };
 
 template <class Ret, class... Args>
 struct coroutine_traits : public traits_sfinae_base<Ret> {};
+// expected-note@-1{{declared here}}
 } // namespace std::experimental
 
 struct suspend_never {
   bool await_ready() noexcept;
   void await_suspend(std::experimental::coroutine_handle<>) noexcept;
   void await_resume() noexcept;
 };
 

clang/test/SemaCXX/coroutine-seh-exp-namespace.cpp

 // RUN: %clang_cc1 -std=c++1z -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -triple x86_64-windows-msvc -fms-extensions
 namespace std::experimental {
 template <typename... T> struct coroutine_traits;
+// expected-note@-1{{declared here}}
 
 template <class Promise = void> struct coroutine_handle {
   coroutine_handle() = default;
   static coroutine_handle from_address(void *) noexcept;
 };
 template <> struct coroutine_handle<void> {
   static coroutine_handle from_address(void *) noexcept;
   coroutine_handle() = default;

clang/test/SemaCXX/coroutine-traits-undefined-template-exp-namespace.cpp

 // This file contains references to sections of the Coroutines TS, which can be
 // found at http://wg21.link/coroutines.
 
 // RUN: %clang_cc1 -std=c++14 -fcoroutines-ts -verify %s -fcxx-exceptions -fexceptions -Wunused-result
 
 namespace std {
 namespace experimental {
 template <typename... T>
-struct coroutine_traits {
+struct coroutine_traits { // expected-note{{declared here}}
   struct promise_type {};
 };
 
 template <> struct coroutine_traits<void>; // expected-note {{forward declaration of 'std::experimental::coroutine_traits<void>'}}
 } // namespace experimental
 } // namespace std
 
 void uses_forward_declaration() {
   co_return; // expected-error {{this function cannot be a coroutine: missing definition of specialization 'coroutine_traits<void>'}}
              // expected-warning@-1 {{support for std::experimental::coroutine_traits will be removed}}
 }

clang/test/SemaCXX/coroutine-unhandled_exception-warning-exp-namespace.cpp

 // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts \
-// RUN:    -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify \
-// RUN:    -fblocks -Wno-unreachable-code -Wno-unused-value
+// RUN:    -fsyntax-only -Wignored-qualifiers -Wno-error=return-type \
+// RUN:    -verify=expected,declared -fblocks -Wno-unreachable-code -Wno-unused-value
 
 // RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts \
 // RUN:    -fsyntax-only -Wignored-qualifiers -Wno-error=return-type -verify \
 // RUN:    -fblocks -Wno-unreachable-code -Wno-unused-value \
 // RUN:    -DDISABLE_WARNING -Wno-deprecated-experimental-coroutine -Wno-coroutine-missing-unhandled-exception
 
 #if __has_feature(cxx_exceptions)
 #error This test requires exceptions be disabled

clang/test/SemaCXX/coroutine_handle-address-return-type-exp-namespace.cpp

 
 template <class T>
 struct traits_sfinae_base<T, void_t<typename T::promise_type>> {
   using promise_type = typename T::promise_type;
 };
 
 template <class Ret, class... Args>
 struct coroutine_traits : public traits_sfinae_base<Ret> {};
+// expected-note@-1{{declared here}}
 } // namespace std::experimental
 
 struct suspend_never {
   bool await_ready() noexcept;
   void await_suspend(std::experimental::coroutine_handle<>) noexcept;
   void await_resume() noexcept;
 };
 

clang/test/SemaCXX/coroutines-exp-namespace.cpp

 
 template <class T>
 struct traits_sfinae_base<T, void_t<typename T::promise_type>> {
   using promise_type = typename T::promise_type;
 };
 
 template <class Ret, class... Args>
 struct coroutine_traits : public traits_sfinae_base<Ret> {};
+// expected-note@-1{{declared here}}
 } // namespace experimental
 } // namespace std
 
 template <typename Promise> struct coro {};
 template <typename Promise, typename... Ps>
 struct std::experimental::coroutine_traits<coro<Promise>, Ps...> {
   using promise_type = Promise;
 };