-
Notifications
You must be signed in to change notification settings - Fork 14.5k
[Clang] Fix export declaration diagnostic message #149059
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Change the error message from "export declaration can only be used within a module purview" to "export declaration can only be used within a module interface" to be technically accurate. The previous message was misleading because export declarations are actually within a module purview when used in module implementation units, but they are only allowed in module interface units. This addresses the issue pointed out in GitHub issue llvm#149008 where @Bigcheese noted that the diagnostic wording was incorrect. The fix updates: - The diagnostic definition in DiagnosticSemaKinds.td - All related test cases to match the new expected error message
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
@llvm/pr-subscribers-clang-modules Author: duhbbx (duhbbx) ChangesChange the error message from "export declaration can only be used within a module purview" to "export declaration can only be used within a module interface" to be technically accurate. The previous message was misleading because export declarations are actually within a module purview when used in module implementation units, but they are only allowed in module interface units. This addresses the issue pointed out in GitHub issue #149008 where @Bigcheese noted that the diagnostic wording was incorrect. The fix updates:
Full diff: https://github.com/llvm/llvm-project/pull/149059.diff 8 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 2781ff81ab4cf..b388c228e5e0f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10642,7 +10642,7 @@ def warn_dangling_reference_captured_by_unknown : Warning<
"the full-expression">, InGroup<DanglingCapture>;
def warn_experimental_lifetime_safety_dummy_warning : Warning<
- "todo: remove this warning after we have atleast one warning based on the lifetime analysis">,
+ "todo: remove this warning after we have atleast one warning based on the lifetime analysis">,
InGroup<LifetimeSafety>, DefaultIgnore;
// For non-floating point, expressions of the form x == x or x != x
@@ -12357,7 +12357,7 @@ def err_export_using_internal : Error<
"using declaration referring to %1 with %select{internal|module|unknown}0 "
"linkage cannot be exported">;
def err_export_not_in_module_interface : Error<
- "export declaration can only be used within a module purview">;
+ "export declaration can only be used within a module interface">;
def err_export_inline_not_defined : Error<
"inline function not defined%select{| before the private module fragment}0">;
def err_export_partition_impl : Error<
@@ -12540,7 +12540,7 @@ def warn_zero_as_null_pointer_constant : Warning<
InGroup<DiagGroup<"zero-as-null-pointer-constant">>, DefaultIgnore;
def warn_not_eliding_copy_on_return : Warning<
- "not eliding copy on return">,
+ "not eliding copy on return">,
InGroup<DiagGroup<"nrvo">>, DefaultIgnore;
def err_nullability_cs_multilevel : Error<
diff --git a/clang/test/CXX/drs/cwg8xx.cpp b/clang/test/CXX/drs/cwg8xx.cpp
index ecb9113ccfe66..7395f04c8e399 100644
--- a/clang/test/CXX/drs/cwg8xx.cpp
+++ b/clang/test/CXX/drs/cwg8xx.cpp
@@ -9,10 +9,10 @@
namespace cwg820 { // cwg820: 2.7
export template <class T> struct B {};
// cxx98-17-warning@-1 {{exported templates are unsupported}}
-// since-cxx20-error@-2 {{export declaration can only be used within a module purview}}
+// since-cxx20-error@-2 {{export declaration can only be used within a module interface}}
export template<typename T> void f() {}
// cxx98-17-warning@-1 {{exported templates are unsupported}}
-// since-cxx20-error@-2 {{export declaration can only be used within a module purview}}
+// since-cxx20-error@-2 {{export declaration can only be used within a module interface}}
} // namespace cwg820
namespace cwg873 { // cwg873: 3.0
diff --git a/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm b/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
index 2158d7fa84b86..ebc76ad16467d 100644
--- a/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
+++ b/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
@@ -9,7 +9,7 @@
//--- ExportDeclNotInModulePurview.cppm
// expected-error@* {{missing 'export module' declaration in module interface unit}}
-export int b; // expected-error {{export declaration can only be used within a module purview}}
+export int b; // expected-error {{export declaration can only be used within a module interface}}
//--- A.cppm
// expected-no-diagnostics
@@ -18,7 +18,7 @@ export int a;
//--- AddExport.cppm
module A; // #module-decl
-export int b; // expected-error {{export declaration can only be used within a module purview}}
+export int b; // expected-error {{export declaration can only be used within a module interface}}
// expected-note@#module-decl {{add 'export' here}}
//--- AddExport2.cppm
diff --git a/clang/test/CXX/module/module.interface/p1.cpp b/clang/test/CXX/module/module.interface/p1.cpp
index c3bfca930f5cc..1754d9ea14618 100644
--- a/clang/test/CXX/module/module.interface/p1.cpp
+++ b/clang/test/CXX/module/module.interface/p1.cpp
@@ -7,7 +7,7 @@
//--- errors.cpp
module;
-export int a; // expected-error {{export declaration can only be used within a module purview}}
+export int a; // expected-error {{export declaration can only be used within a module interface}}
export module M;
export int b; // #1
namespace N {
@@ -37,8 +37,8 @@ namespace N {
//--- impl.cpp
module M; // #M
-export int b2; // expected-error {{export declaration can only be used within a module purview}}
+export int b2; // expected-error {{export declaration can only be used within a module interface}}
namespace N {
- export int c2; // expected-error {{export declaration can only be used within a module purview}}
+ export int c2; // expected-error {{export declaration can only be used within a module interface}}
}
// expected-note@#M 2+{{add 'export'}}
diff --git a/clang/test/Modules/cxx20-10-2-ex1.cpp b/clang/test/Modules/cxx20-10-2-ex1.cpp
index 0cd6f77466f4b..749b15213098a 100644
--- a/clang/test/Modules/cxx20-10-2-ex1.cpp
+++ b/clang/test/Modules/cxx20-10-2-ex1.cpp
@@ -14,7 +14,7 @@ export int x;
module;
#include "std-10-2-ex1.h"
-// [email protected]:* {{export declaration can only be used within a module purview}}
+// [email protected]:* {{export declaration can only be used within a module interface}}
export module M1;
export namespace {} // expected-error {{anonymous namespaces cannot be exported}}
diff --git a/clang/test/Modules/cxx20-export-import.cpp b/clang/test/Modules/cxx20-export-import.cpp
index 0b505668e8589..c14883e575575 100644
--- a/clang/test/Modules/cxx20-export-import.cpp
+++ b/clang/test/Modules/cxx20-export-import.cpp
@@ -11,4 +11,4 @@
export module dummy;
//--- test.cpp
-export import dummy; // expected-error {{export declaration can only be used within a module purview}}
+export import dummy; // expected-error {{export declaration can only be used within a module interface}}
diff --git a/clang/test/Modules/cxx20-import-diagnostics-a.cpp b/clang/test/Modules/cxx20-import-diagnostics-a.cpp
index 1b38259e0358c..72a31ea1d7d78 100644
--- a/clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ b/clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -110,7 +110,7 @@ module;
module AOK1;
-export import C; // expected-error {{export declaration can only be used within a module purview}}
+export import C; // expected-error {{export declaration can only be used within a module interface}}
int theAnswer () { return 42; }
diff --git a/clang/test/Modules/export-in-non-modules.cpp b/clang/test/Modules/export-in-non-modules.cpp
index 69360eb46d774..7b2575c60f1fd 100644
--- a/clang/test/Modules/export-in-non-modules.cpp
+++ b/clang/test/Modules/export-in-non-modules.cpp
@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
-export struct Unit { // expected-error {{export declaration can only be used within a module purview}}
+export struct Unit { // expected-error {{export declaration can only be used within a module interface}}
bool operator<(const Unit &);
};
|
@llvm/pr-subscribers-clang Author: duhbbx (duhbbx) ChangesChange the error message from "export declaration can only be used within a module purview" to "export declaration can only be used within a module interface" to be technically accurate. The previous message was misleading because export declarations are actually within a module purview when used in module implementation units, but they are only allowed in module interface units. This addresses the issue pointed out in GitHub issue #149008 where @Bigcheese noted that the diagnostic wording was incorrect. The fix updates:
Full diff: https://github.com/llvm/llvm-project/pull/149059.diff 8 Files Affected:
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 2781ff81ab4cf..b388c228e5e0f 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10642,7 +10642,7 @@ def warn_dangling_reference_captured_by_unknown : Warning<
"the full-expression">, InGroup<DanglingCapture>;
def warn_experimental_lifetime_safety_dummy_warning : Warning<
- "todo: remove this warning after we have atleast one warning based on the lifetime analysis">,
+ "todo: remove this warning after we have atleast one warning based on the lifetime analysis">,
InGroup<LifetimeSafety>, DefaultIgnore;
// For non-floating point, expressions of the form x == x or x != x
@@ -12357,7 +12357,7 @@ def err_export_using_internal : Error<
"using declaration referring to %1 with %select{internal|module|unknown}0 "
"linkage cannot be exported">;
def err_export_not_in_module_interface : Error<
- "export declaration can only be used within a module purview">;
+ "export declaration can only be used within a module interface">;
def err_export_inline_not_defined : Error<
"inline function not defined%select{| before the private module fragment}0">;
def err_export_partition_impl : Error<
@@ -12540,7 +12540,7 @@ def warn_zero_as_null_pointer_constant : Warning<
InGroup<DiagGroup<"zero-as-null-pointer-constant">>, DefaultIgnore;
def warn_not_eliding_copy_on_return : Warning<
- "not eliding copy on return">,
+ "not eliding copy on return">,
InGroup<DiagGroup<"nrvo">>, DefaultIgnore;
def err_nullability_cs_multilevel : Error<
diff --git a/clang/test/CXX/drs/cwg8xx.cpp b/clang/test/CXX/drs/cwg8xx.cpp
index ecb9113ccfe66..7395f04c8e399 100644
--- a/clang/test/CXX/drs/cwg8xx.cpp
+++ b/clang/test/CXX/drs/cwg8xx.cpp
@@ -9,10 +9,10 @@
namespace cwg820 { // cwg820: 2.7
export template <class T> struct B {};
// cxx98-17-warning@-1 {{exported templates are unsupported}}
-// since-cxx20-error@-2 {{export declaration can only be used within a module purview}}
+// since-cxx20-error@-2 {{export declaration can only be used within a module interface}}
export template<typename T> void f() {}
// cxx98-17-warning@-1 {{exported templates are unsupported}}
-// since-cxx20-error@-2 {{export declaration can only be used within a module purview}}
+// since-cxx20-error@-2 {{export declaration can only be used within a module interface}}
} // namespace cwg820
namespace cwg873 { // cwg873: 3.0
diff --git a/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm b/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
index 2158d7fa84b86..ebc76ad16467d 100644
--- a/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
+++ b/clang/test/CXX/module/dcl.dcl/dcl.module/dcl.module.interface/p1.cppm
@@ -9,7 +9,7 @@
//--- ExportDeclNotInModulePurview.cppm
// expected-error@* {{missing 'export module' declaration in module interface unit}}
-export int b; // expected-error {{export declaration can only be used within a module purview}}
+export int b; // expected-error {{export declaration can only be used within a module interface}}
//--- A.cppm
// expected-no-diagnostics
@@ -18,7 +18,7 @@ export int a;
//--- AddExport.cppm
module A; // #module-decl
-export int b; // expected-error {{export declaration can only be used within a module purview}}
+export int b; // expected-error {{export declaration can only be used within a module interface}}
// expected-note@#module-decl {{add 'export' here}}
//--- AddExport2.cppm
diff --git a/clang/test/CXX/module/module.interface/p1.cpp b/clang/test/CXX/module/module.interface/p1.cpp
index c3bfca930f5cc..1754d9ea14618 100644
--- a/clang/test/CXX/module/module.interface/p1.cpp
+++ b/clang/test/CXX/module/module.interface/p1.cpp
@@ -7,7 +7,7 @@
//--- errors.cpp
module;
-export int a; // expected-error {{export declaration can only be used within a module purview}}
+export int a; // expected-error {{export declaration can only be used within a module interface}}
export module M;
export int b; // #1
namespace N {
@@ -37,8 +37,8 @@ namespace N {
//--- impl.cpp
module M; // #M
-export int b2; // expected-error {{export declaration can only be used within a module purview}}
+export int b2; // expected-error {{export declaration can only be used within a module interface}}
namespace N {
- export int c2; // expected-error {{export declaration can only be used within a module purview}}
+ export int c2; // expected-error {{export declaration can only be used within a module interface}}
}
// expected-note@#M 2+{{add 'export'}}
diff --git a/clang/test/Modules/cxx20-10-2-ex1.cpp b/clang/test/Modules/cxx20-10-2-ex1.cpp
index 0cd6f77466f4b..749b15213098a 100644
--- a/clang/test/Modules/cxx20-10-2-ex1.cpp
+++ b/clang/test/Modules/cxx20-10-2-ex1.cpp
@@ -14,7 +14,7 @@ export int x;
module;
#include "std-10-2-ex1.h"
-// [email protected]:* {{export declaration can only be used within a module purview}}
+// [email protected]:* {{export declaration can only be used within a module interface}}
export module M1;
export namespace {} // expected-error {{anonymous namespaces cannot be exported}}
diff --git a/clang/test/Modules/cxx20-export-import.cpp b/clang/test/Modules/cxx20-export-import.cpp
index 0b505668e8589..c14883e575575 100644
--- a/clang/test/Modules/cxx20-export-import.cpp
+++ b/clang/test/Modules/cxx20-export-import.cpp
@@ -11,4 +11,4 @@
export module dummy;
//--- test.cpp
-export import dummy; // expected-error {{export declaration can only be used within a module purview}}
+export import dummy; // expected-error {{export declaration can only be used within a module interface}}
diff --git a/clang/test/Modules/cxx20-import-diagnostics-a.cpp b/clang/test/Modules/cxx20-import-diagnostics-a.cpp
index 1b38259e0358c..72a31ea1d7d78 100644
--- a/clang/test/Modules/cxx20-import-diagnostics-a.cpp
+++ b/clang/test/Modules/cxx20-import-diagnostics-a.cpp
@@ -110,7 +110,7 @@ module;
module AOK1;
-export import C; // expected-error {{export declaration can only be used within a module purview}}
+export import C; // expected-error {{export declaration can only be used within a module interface}}
int theAnswer () { return 42; }
diff --git a/clang/test/Modules/export-in-non-modules.cpp b/clang/test/Modules/export-in-non-modules.cpp
index 69360eb46d774..7b2575c60f1fd 100644
--- a/clang/test/Modules/export-in-non-modules.cpp
+++ b/clang/test/Modules/export-in-non-modules.cpp
@@ -1,4 +1,4 @@
// RUN: %clang_cc1 -std=c++20 %s -fsyntax-only -verify
-export struct Unit { // expected-error {{export declaration can only be used within a module purview}}
+export struct Unit { // expected-error {{export declaration can only be used within a module interface}}
bool operator<(const Unit &);
};
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes to DR tests LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo the whitespace-only changes
@@ -10642,7 +10642,7 @@ def warn_dangling_reference_captured_by_unknown : Warning< | |||
"the full-expression">, InGroup<DanglingCapture>; | |||
|
|||
def warn_experimental_lifetime_safety_dummy_warning : Warning< | |||
"todo: remove this warning after we have atleast one warning based on the lifetime analysis">, | |||
"todo: remove this warning after we have atleast one warning based on the lifetime analysis">, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you revert this whitespace change?
@@ -12540,7 +12540,7 @@ def warn_zero_as_null_pointer_constant : Warning< | |||
InGroup<DiagGroup<"zero-as-null-pointer-constant">>, DefaultIgnore; | |||
|
|||
def warn_not_eliding_copy_on_return : Warning< | |||
"not eliding copy on return">, | |||
"not eliding copy on return">, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you revert this whitespace change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM modulo whitespaces changes.
Thanks for the fix
Should I submit another PR to convert the spaces in these two places? @cor3ntin |
No, please add a commit to your branch (and push), it will update this PR! Thanks |
This reverts unintended whitespace removal caused by IDE auto-formatting in commits: - llvm#149059 ("Fix export declaration diagnostic message")
Thanks! I'll merge this later today, unless @Bigcheese has opinions |
@duhbbx Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/51/builds/19953 Here is the relevant piece of the build log for the reference
|
Change the error message from "export declaration can only be used within a module purview" to "export declaration can only be used within a module interface" to be technically accurate.
The previous message was misleading because export declarations are actually within a module purview when used in module implementation units, but they are only allowed in module interface units.
This addresses the issue pointed out in GitHub issue #149008 where Bigcheese noted that the diagnostic wording was incorrect.
Fixes #149008