Re: [PATCH] D33660: [coroutines] Fix assertion during -Wuninitialized analysis

[Eric Fiselier via cfe-commits]

Yes, the thing that it's supposed to be testing is -Wuninitialized and
other analysis warnings.

I'll re-name the test and add more test cases to this effect shortly.
Thanks for the input.

On Mon, Jun 5, 2017 at 10:33 AM, David Blaikie  wrote:

>
>
> On Mon, May 29, 2017 at 3:28 PM Eric Fiselier via Phabricator via
> cfe-commits  wrote:
>
>> EricWF created this revision.
>>
>> @rsmith Is there a better place to put this test?
>>
>>
>> https://reviews.llvm.org/D33660
>>
>> Files:
>>   lib/Sema/SemaCoroutine.cpp
>>   test/SemaCXX/coreturn.cpp
>>   test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
>>
>>
>> Index: test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
>> ===
>> --- /dev/null
>> +++ test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
>> @@ -0,0 +1,44 @@
>> +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14
>> -fcoroutines-ts -fsyntax-only -Wall -Wextra -Wuninitialized  -fblocks
>>
>
> A test case that tests only "this doesn't crash" is usually a bit of a
> hint to me, at least, that something's under-tested. I assume there's some
> specific behavior that's desired more than "does anything other than
> crashing" that should be being tested for here?
>
>
>> +#include "Inputs/std-coroutine.h"
>> +
>> +using namespace std::experimental;
>> +
>> +
>> +struct A {
>> +  bool await_ready() { return true; }
>> +  int await_resume() { return 42; }
>> +  template 
>> +  void await_suspend(F) {}
>> +};
>> +
>> +
>> +struct coro_t {
>> +  struct promise_type {
>> +coro_t get_return_object() { return {}; }
>> +suspend_never initial_suspend() { return {}; }
>> +suspend_never final_suspend() { return {}; }
>> +A yield_value(int) { return {}; }
>> +void return_void() {}
>> +static void unhandled_exception() {}
>> +  };
>> +};
>> +
>> +coro_t f(int n) {
>> +  if (n == 0)
>> +co_return;
>> +  co_yield 42;
>> +  int x = co_await A{};
>> +}
>> +
>> +template 
>> +coro_t g(int n) {
>> +  if (n == 0)
>> +co_return;
>> +  co_yield 42;
>> +  int x = co_await Await{};
>> +}
>> +
>> +int main() {
>> +  f(0);
>> +  g(0);
>> +}
>> Index: test/SemaCXX/coreturn.cpp
>> ===
>> --- test/SemaCXX/coreturn.cpp
>> +++ test/SemaCXX/coreturn.cpp
>> @@ -1,4 +1,4 @@
>> -// 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 -fblocks -Wall -Wextra -Wno-error=unreachable-code
>>  #include "Inputs/std-coroutine.h"
>>
>>  using std::experimental::suspend_always;
>> Index: lib/Sema/SemaCoroutine.cpp
>> ===
>> --- lib/Sema/SemaCoroutine.cpp
>> +++ lib/Sema/SemaCoroutine.cpp
>> @@ -437,6 +437,7 @@
>>if (VD->isInvalidDecl())
>>  return nullptr;
>>ActOnUninitializedDecl(VD);
>> +  FD->addDecl(VD);
>>assert(!VD->isInvalidDecl());
>>return VD;
>>  }
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D33660: [coroutines] Fix assertion during -Wuninitialized analysis

[David Blaikie via cfe-commits]

On Mon, May 29, 2017 at 3:28 PM Eric Fiselier via Phabricator via
cfe-commits  wrote:

> EricWF created this revision.
>
> @rsmith Is there a better place to put this test?
>
>
> https://reviews.llvm.org/D33660
>
> Files:
>   lib/Sema/SemaCoroutine.cpp
>   test/SemaCXX/coreturn.cpp
>   test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
>
>
> Index: test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
> ===
> --- /dev/null
> +++ test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
> @@ -0,0 +1,44 @@
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14
> -fcoroutines-ts -fsyntax-only -Wall -Wextra -Wuninitialized  -fblocks
>

A test case that tests only "this doesn't crash" is usually a bit of a hint
to me, at least, that something's under-tested. I assume there's some
specific behavior that's desired more than "does anything other than
crashing" that should be being tested for here?


> +#include "Inputs/std-coroutine.h"
> +
> +using namespace std::experimental;
> +
> +
> +struct A {
> +  bool await_ready() { return true; }
> +  int await_resume() { return 42; }
> +  template 
> +  void await_suspend(F) {}
> +};
> +
> +
> +struct coro_t {
> +  struct promise_type {
> +coro_t get_return_object() { return {}; }
> +suspend_never initial_suspend() { return {}; }
> +suspend_never final_suspend() { return {}; }
> +A yield_value(int) { return {}; }
> +void return_void() {}
> +static void unhandled_exception() {}
> +  };
> +};
> +
> +coro_t f(int n) {
> +  if (n == 0)
> +co_return;
> +  co_yield 42;
> +  int x = co_await A{};
> +}
> +
> +template 
> +coro_t g(int n) {
> +  if (n == 0)
> +co_return;
> +  co_yield 42;
> +  int x = co_await Await{};
> +}
> +
> +int main() {
> +  f(0);
> +  g(0);
> +}
> Index: test/SemaCXX/coreturn.cpp
> ===
> --- test/SemaCXX/coreturn.cpp
> +++ test/SemaCXX/coreturn.cpp
> @@ -1,4 +1,4 @@
> -// 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 -fblocks -Wall -Wextra -Wno-error=unreachable-code
>  #include "Inputs/std-coroutine.h"
>
>  using std::experimental::suspend_always;
> Index: lib/Sema/SemaCoroutine.cpp
> ===
> --- lib/Sema/SemaCoroutine.cpp
> +++ lib/Sema/SemaCoroutine.cpp
> @@ -437,6 +437,7 @@
>if (VD->isInvalidDecl())
>  return nullptr;
>ActOnUninitializedDecl(VD);
> +  FD->addDecl(VD);
>assert(!VD->isInvalidDecl());
>return VD;
>  }
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33660: [coroutines] Fix assertion during -Wuninitialized analysis

[Gor Nishanov via Phabricator via cfe-commits]

GorNishanov accepted this revision.
GorNishanov added a comment.
This revision is now accepted and ready to land.

LBTM (Looks brilliant to me)


https://reviews.llvm.org/D33660



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

[PATCH] D33660: [coroutines] Fix assertion during -Wuninitialized analysis

[Eric Fiselier via Phabricator via cfe-commits]

EricWF created this revision.

@rsmith Is there a better place to put this test?


https://reviews.llvm.org/D33660

Files:
  lib/Sema/SemaCoroutine.cpp
  test/SemaCXX/coreturn.cpp
  test/SemaCXX/coroutine-uninitialized-warning-crash.cpp


Index: test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
===
--- /dev/null
+++ test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts 
-fsyntax-only -Wall -Wextra -Wuninitialized  -fblocks
+#include "Inputs/std-coroutine.h"
+
+using namespace std::experimental;
+
+
+struct A {
+  bool await_ready() { return true; }
+  int await_resume() { return 42; }
+  template 
+  void await_suspend(F) {}
+};
+
+
+struct coro_t {
+  struct promise_type {
+coro_t get_return_object() { return {}; }
+suspend_never initial_suspend() { return {}; }
+suspend_never final_suspend() { return {}; }
+A yield_value(int) { return {}; }
+void return_void() {}
+static void unhandled_exception() {}
+  };
+};
+
+coro_t f(int n) {
+  if (n == 0)
+co_return;
+  co_yield 42;
+  int x = co_await A{};
+}
+
+template 
+coro_t g(int n) {
+  if (n == 0)
+co_return;
+  co_yield 42;
+  int x = co_await Await{};
+}
+
+int main() {
+  f(0);
+  g(0);
+}
Index: test/SemaCXX/coreturn.cpp
===
--- test/SemaCXX/coreturn.cpp
+++ test/SemaCXX/coreturn.cpp
@@ -1,4 +1,4 @@
-// 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 -fblocks 
-Wall -Wextra -Wno-error=unreachable-code
 #include "Inputs/std-coroutine.h"
 
 using std::experimental::suspend_always;
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -437,6 +437,7 @@
   if (VD->isInvalidDecl())
 return nullptr;
   ActOnUninitializedDecl(VD);
+  FD->addDecl(VD);
   assert(!VD->isInvalidDecl());
   return VD;
 }


Index: test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
===
--- /dev/null
+++ test/SemaCXX/coroutine-uninitialized-warning-crash.cpp
@@ -0,0 +1,44 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin9 %s -std=c++14 -fcoroutines-ts -fsyntax-only -Wall -Wextra -Wuninitialized  -fblocks
+#include "Inputs/std-coroutine.h"
+
+using namespace std::experimental;
+
+
+struct A {
+  bool await_ready() { return true; }
+  int await_resume() { return 42; }
+  template 
+  void await_suspend(F) {}
+};
+
+
+struct coro_t {
+  struct promise_type {
+coro_t get_return_object() { return {}; }
+suspend_never initial_suspend() { return {}; }
+suspend_never final_suspend() { return {}; }
+A yield_value(int) { return {}; }
+void return_void() {}
+static void unhandled_exception() {}
+  };
+};
+
+coro_t f(int n) {
+  if (n == 0)
+co_return;
+  co_yield 42;
+  int x = co_await A{};
+}
+
+template 
+coro_t g(int n) {
+  if (n == 0)
+co_return;
+  co_yield 42;
+  int x = co_await Await{};
+}
+
+int main() {
+  f(0);
+  g(0);
+}
Index: test/SemaCXX/coreturn.cpp
===
--- test/SemaCXX/coreturn.cpp
+++ test/SemaCXX/coreturn.cpp
@@ -1,4 +1,4 @@
-// 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 -fblocks -Wall -Wextra -Wno-error=unreachable-code
 #include "Inputs/std-coroutine.h"
 
 using std::experimental::suspend_always;
Index: lib/Sema/SemaCoroutine.cpp
===
--- lib/Sema/SemaCoroutine.cpp
+++ lib/Sema/SemaCoroutine.cpp
@@ -437,6 +437,7 @@
   if (VD->isInvalidDecl())
 return nullptr;
   ActOnUninitializedDecl(VD);
+  FD->addDecl(VD);
   assert(!VD->isInvalidDecl());
   return VD;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits