Add missing `struct` keyword to the test p2-2.cpp

[ayzhao]

Summary

While working on D53847, I noticed that this test would fail once we
started recognizing the types in the modified export statement [0].
The tests would fail because Clang would emit a "declaration does not
declare anything" diagnostic instead of the expected namespace scope
diagnostic.

I believe that the test is currently incorrectly passing because Clang
doesn't parse the type and therefore doesn't treat the statement as a
declaration. My understanding is that the intention of this test case is
that it wants to export a struct type, which I believe requires a
struct keyword, even for types with template parameters. With this
change, the only error with these two statements should be the
namespace scope issue.

[0]: https://reviews.llvm.org/D53847?id=462032#inline-1297053

[ilya-biryukov]

LGTM. I believe the standard is clear here, the declaration inside export does not have any special treatment in terms of how it must be parsed.

@ChuanqiXu could you confirm my reasoning?

Two more related rants follow.

It is unclear why the test did not produce any errors about these forward declarations being invalid in the first place.
Forward declaration cannot have a nested name specifier, even if all export restrictions are met, we will still get the error.
I suppose we aim to produce fewer errors here, so showing only one of the two errors makes sense. However, I feel that the preference should be given to "invalid forward declaration" as it occurs "deeper" and makes the exported construct invalid in the first place. Hence, any attempt to interpret the invalid exported declaration can mask the original error. What do others think?

The standard says in (module.interface)p3: An exported declaration that is not a module-import-declaration shall declare at least one name. Should the "declaration does not declare anything" be promoted to error when inside export declaration?

[erichkeane]

This comment was removed by erichkeane.

[erichkeane]

Er... please dont' count that 'accept', let @ChuanqiXu be the approver here.

[ilya-biryukov]

Is this the only thing that blocks D53847?
I suggest to stamp this if so (happy to do it myself). In case @ChuanqiXu will have comments we can address them in post-commit review.

[ayzhao]

In D134578#3821178, @ilya-biryukov wrote:

Is this the only thing that blocks D53847?
I suggest to stamp this if so (happy to do it myself). In case @ChuanqiXu will have comments we can address them in post-commit review.

SG, I'll go ahead and land this and D53847.

TBH this patch was probably trivial enough to not require

[aaron.ballman]

In D134578#3821301, @ayzhao wrote:

In D134578#3821178, @ilya-biryukov wrote:

Is this the only thing that blocks D53847?
I suggest to stamp this if so (happy to do it myself). In case @ChuanqiXu will have comments we can address them in post-commit review.

SG, I'll go ahead and land this and D53847.

TBH this patch was probably trivial enough to not require

FWIW, the changes here LGTM and I agree that this probably didn't require a review given the trivial nature of the changes.

clang/test/CXX/module/module.interface/p2-2.cpp

 // The intention of this file to check we could only export declarations in namesapce scope.
 //
 // RUN: %clang_cc1 -std=c++20 %s -verify
 
 export module X;
 
 export template <typename T>
 struct X {
   struct iterator {
     T node;
   };
   void foo() {}
   template <typename U>
   U bar();
 };
 
-export template <typename T> X<T>::iterator;                      // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+export template <typename T> struct X<T>::iterator;               // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
 export template <typename T> void X<T>::foo();                    // expected-error {{cannot export 'foo' as it is not at namespace scope}}
 export template <typename T> template <typename U> U X<T>::bar(); // expected-error {{cannot export 'bar' as it is not at namespace scope}}
 
 export struct Y {
   struct iterator {
     int node;
   };
   void foo() {}
   template <typename U>
   U bar();
 };
 
 export struct Y::iterator;               // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
 export void Y::foo();                    // expected-error {{cannot export 'foo' as it is not at namespace scope}}
 export template <typename U> U Y::bar(); // expected-error {{cannot export 'bar' as it is not at namespace scope}}
 
 export {
-  template <typename T> X<T>::iterator; // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
-  struct Y::iterator;                   // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+  template <typename T> struct X<T>::iterator; // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
+  struct Y::iterator;                          // expected-error {{cannot export 'iterator' as it is not at namespace scope}}
 }