- Notifications
You must be signed in to change notification settings - Fork14.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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
Change the error message from "export declaration can only be usedwithin a module purview" to "export declaration can only be usedwithin a module interface" to be technically accurate.The previous message was misleading because export declarationsare actually within a module purview when used in module implementationunits, but they are only allowed in module interface units.This addresses the issue pointed out in GitHub issuellvm#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 theLLVM GitHub User Guide. You can also ask questions in a comment on this PR, on theLLVM Discord or on theforums. |
@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.tdindex 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.cppindex 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.0diff --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.cppmindex 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.cppmdiff --git a/clang/test/CXX/module/module.interface/p1.cpp b/clang/test/CXX/module/module.interface/p1.cppindex 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.cppindex 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"-// expected-error@std-10-2-ex1.h:* {{export declaration can only be used within a module purview}}+// expected-error@std-10-2-ex1.h:* {{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.cppindex 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.cppindex 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.cppindex 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.tdindex 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.cppindex 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.0diff --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.cppmindex 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.cppmdiff --git a/clang/test/CXX/module/module.interface/p1.cpp b/clang/test/CXX/module/module.interface/p1.cppindex 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.cppindex 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"-// expected-error@std-10-2-ex1.h:* {{export declaration can only be used within a module purview}}+// expected-error@std-10-2-ex1.h:* {{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.cppindex 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.cppindex 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.cppindex 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
cor3ntin approved these changesJul 16, 2025
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?
cor3ntin approved these changesJul 16, 2025
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-formattingin commits:-llvm#149059 ("Fix export declaration diagnostic message")
Thanks! I'll merge this later today, unless@Bigcheese has opinions |
Bigcheese approved these changesJul 16, 2025
fdec9fd intollvm:main 9 checks passed
Uh oh!
There was an error while loading.Please reload this page.
@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 ourbuild 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 detailhere. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part ofLLVM 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 |
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
clang:frontendLanguage frontend issues, e.g. anything involving "Sema" clang:modulesC++20 modules and Clang Header Modules clangClang issues not falling into any other category
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading.Please reload this page.
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