Skip to content
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

[libc++][modules] Fixes clang-tidy exports. #76288

Merged
merged 1 commit into from
Jan 17, 2024

Conversation

mordante
Copy link
Member

@mordante mordante commented Dec 23, 2023

As suggested in #71438 we should use
export import std;
in the std.compat module.

Using this exports some named declarations from functions and records, adding them to the global namespace. Clang correctly does not export these and it's an issue in the declaration filtering. Declarations in function or record context are not considered a global named declaration.

@mordante mordante requested a review from a team as a code owner December 23, 2023 14:31
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Dec 23, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Dec 23, 2023

@llvm/pr-subscribers-libcxx

Author: Mark de Wever (mordante)

Changes

As suggested in #71438 we should use
export import std;
in the std.compat module.

Using this exports some named declarations from functions and records, adding them to the global namespace. Clang correctly, does not export these it's and issue in the declaration filtering. Declarations in function or record context are not considered a global named declaration.


Full diff: https://github.com/llvm/llvm-project/pull/76288.diff

1 Files Affected:

  • (modified) libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp (+5-1)
diff --git a/libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp b/libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp
index 35f020da45c435..15d2e6839ad5e3 100644
--- a/libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp
+++ b/libcxx/test/tools/clang_tidy_checks/header_exportable_declarations.cpp
@@ -252,9 +252,13 @@ static bool is_global_name_exported_by_std_module(std::string_view name) {
 
 static bool is_valid_declaration_context(
     const clang::NamedDecl& decl, std::string_view name, header_exportable_declarations::FileType file_type) {
-  if (decl.getDeclContext()->isNamespace())
+  const clang::DeclContext& context = *decl.getDeclContext();
+  if (context.isNamespace())
     return true;
 
+  if (context.isFunctionOrMethod() || context.isRecord())
+    return false;
+
   if (is_global_name_exported_by_std_module(name))
     return true;
 

@mordante mordante force-pushed the users/mordante/switches_to_clang-tidy-18 branch 2 times, most recently from b3c567f to 84c74e0 Compare January 17, 2024 07:18
Base automatically changed from users/mordante/switches_to_clang-tidy-18 to main January 17, 2024 07:18
As suggested in #71438 we should use
  export import std;
in the std.compat module.

Using this exports some named declarations from functions and records,
adding them to the global namespace. Clang correctly, does not export
these it's and issue in the declaration filtering. Declarations in
function or record context are not considered a global named
declaration.
@mordante mordante force-pushed the users/mordante/fixes_clang-tidy_exports branch from dd147d3 to 3d82f80 Compare January 17, 2024 07:26
@mordante mordante merged commit fb79f80 into main Jan 17, 2024
7 of 10 checks passed
@mordante mordante deleted the users/mordante/fixes_clang-tidy_exports branch January 17, 2024 07:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants