[PATCH] c++/modules: fix up recent testcases

[PATCH] c++/modules: fix up recent testcases

[Patrick Palka]

Tested on x86_64-pc-linux-gnu, does this look OK for trunk?

Declaring get() inline seems necessary to avoid link failure:

  /usr/bin/ld: /tmp/ccwdv6Co.o: in function `g3@pr105322.Decltype()':
  decltype-1_b.C:(.text._ZW8pr105322W8Decltype2g3v[_ZW8pr105322W8Decltype2g3v]+0x18): undefined reference to `f@pr105322.Decltype()::A::get()'

Not sure if that's expected?

-- >8 --

This fixes some minor issues with the testcases from
r14-4806-g084addf8a700fa.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/decltype-1_a.C: Add missing } to dg-module-do
	directive.  Declare f()::A::get() inline.
	* g++.dg/modules/lambda-5_a.C: Add missing } to dg-module-do
	directive.
---
 gcc/testsuite/g++.dg/modules/decltype-1_a.C | 4 ++--
 gcc/testsuite/g++.dg/modules/lambda-5_a.C   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_a.C b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
index ca66e8b598a..6512f151aae 100644
--- a/gcc/testsuite/g++.dg/modules/decltype-1_a.C
+++ b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
@@ -1,5 +1,5 @@
 // PR c++/105322
-// { dg-module-do link
+// { dg-module-do link }
 // { dg-additional-options -fmodules-ts }
 // { dg-module-cmi pr105322.Decltype }
 
@@ -7,7 +7,7 @@ export module pr105322.Decltype;
 
 auto f() {
   struct A { int m;
-    int get () { return m; }
+    inline int get () { return m; }
   };
   return A{};
 }
diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
index 6b589d4965c..37d0e77b1e1 100644
--- a/gcc/testsuite/g++.dg/modules/lambda-5_a.C
+++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
@@ -1,5 +1,5 @@
 // PR c++/105322
-// { dg-module-do link
+// { dg-module-do link }
 // { dg-additional-options -fmodules-ts }
 // { dg-module-cmi pr105322.Lambda }
 
-- 
2.42.0.482.g2e8e77cbac

Re: [PATCH] c++/modules: fix up recent testcases

[Nathan Sidwell]

Patrick,

thanks for noticing this, and this is a suitable workaround for another bug.

We should either be emitting the definition of that member function in the 
object file of its containing function.  Or it should be implicitly inline.

I know in module perview the in-class defined member functions at namespace 
scope are /not/ implicitly inline.  But I can't recall what the std says about 
non-namespace scope classes.

Ah, it appears to be the former we should be doing:

[class.mfct] If a member function is attached to the global module and is 
defined (9.5) in its class definition, it is inline (9.2.8).

Notice we can get into the weird situation that the member functions of a local 
class of a module-purview inline function might not themselves be inline!


On 10/25/23 14:32, Patrick Palka wrote:
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> 
> Declaring get() inline seems necessary to avoid link failure:
> 
>    /usr/bin/ld: /tmp/ccwdv6Co.o: in function `g3@pr105322.Decltype()':
>    decltype-1_b.C:(.text._ZW8pr105322W8Decltype2g3v[_ZW8pr105322W8Decltype2g3v]+0x18): undefined reference to `f@pr105322.Decltype()::A::get()'
> 
> Not sure if that's expected?
> 
> -- >8 --
> 
> This fixes some minor issues with the testcases from
> r14-4806-g084addf8a700fa.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/decltype-1_a.C: Add missing } to dg-module-do
> 	directive.  Declare f()::A::get() inline.
> 	* g++.dg/modules/lambda-5_a.C: Add missing } to dg-module-do
> 	directive.
> ---
>   gcc/testsuite/g++.dg/modules/decltype-1_a.C | 4 ++--
>   gcc/testsuite/g++.dg/modules/lambda-5_a.C   | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_a.C b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
> index ca66e8b598a..6512f151aae 100644
> --- a/gcc/testsuite/g++.dg/modules/decltype-1_a.C
> +++ b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
> @@ -1,5 +1,5 @@
>   // PR c++/105322
> -// { dg-module-do link
> +// { dg-module-do link }
>   // { dg-additional-options -fmodules-ts }
>   // { dg-module-cmi pr105322.Decltype }
>   
> @@ -7,7 +7,7 @@ export module pr105322.Decltype;
>   
>   auto f() {
>     struct A { int m;
> -    int get () { return m; }
> +    inline int get () { return m; }
>     };
>     return A{};
>   }
> diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> index 6b589d4965c..37d0e77b1e1 100644
> --- a/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> @@ -1,5 +1,5 @@
>   // PR c++/105322
> -// { dg-module-do link
> +// { dg-module-do link }
>   // { dg-additional-options -fmodules-ts }
>   // { dg-module-cmi pr105322.Lambda }
>   

-- 
Nathan Sidwell

Re: [PATCH] c++/modules: fix up recent testcases

[Jason Merrill]

On 10/25/23 14:32, Patrick Palka wrote:
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
> 
> Declaring get() inline seems necessary to avoid link failure:
> 
>    /usr/bin/ld: /tmp/ccwdv6Co.o: in function `g3@pr105322.Decltype()':
>    decltype-1_b.C:(.text._ZW8pr105322W8Decltype2g3v[_ZW8pr105322W8Decltype2g3v]+0x18): undefined reference to `f@pr105322.Decltype()::A::get()'
> 
> Not sure if that's expected?

That seems like a bug.  My guess is that the bug is treating f()::A::get 
as internal linkage, but Nathan should have a better understanding of 
how it's supposed to work.

> -- >8 --
> 
> This fixes some minor issues with the testcases from
> r14-4806-g084addf8a700fa.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/decltype-1_a.C: Add missing } to dg-module-do
> 	directive.  Declare f()::A::get() inline.
> 	* g++.dg/modules/lambda-5_a.C: Add missing } to dg-module-do
> 	directive.
> ---
>   gcc/testsuite/g++.dg/modules/decltype-1_a.C | 4 ++--
>   gcc/testsuite/g++.dg/modules/lambda-5_a.C   | 2 +-
>   2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_a.C b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
> index ca66e8b598a..6512f151aae 100644
> --- a/gcc/testsuite/g++.dg/modules/decltype-1_a.C
> +++ b/gcc/testsuite/g++.dg/modules/decltype-1_a.C
> @@ -1,5 +1,5 @@
>   // PR c++/105322
> -// { dg-module-do link
> +// { dg-module-do link }
>   // { dg-additional-options -fmodules-ts }
>   // { dg-module-cmi pr105322.Decltype }
>   
> @@ -7,7 +7,7 @@ export module pr105322.Decltype;
>   
>   auto f() {
>     struct A { int m;
> -    int get () { return m; }
> +    inline int get () { return m; }
>     };
>     return A{};
>   }
> diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> index 6b589d4965c..37d0e77b1e1 100644
> --- a/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C
> @@ -1,5 +1,5 @@
>   // PR c++/105322
> -// { dg-module-do link
> +// { dg-module-do link }
>   // { dg-additional-options -fmodules-ts }
>   // { dg-module-cmi pr105322.Lambda }
>