Kaynağa Gözat

Reverse Interop: Nested namespace support (#6940)

Start recording the clang::DeclContext* -> InstId mapping for use in
later operations.

The test update includes removing the initial fail_* test because I
hadn't thought about the use of namespace aliases as a way to test for
the presence of a namespace without the failure caused by not finding
the thing inside the namespace.

---------

Co-authored-by: Jon Ross-Perkins <jperkins@google.com>
David Blaikie 1 ay önce
ebeveyn
işleme
2af5f971da

+ 55 - 20
toolchain/check/cpp/generate_ast.cpp

@@ -20,6 +20,7 @@
 #include "clang/Sema/MultiplexExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "common/check.h"
+#include "common/map.h"
 #include "common/raw_string_ostream.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/StringRef.h"
@@ -306,22 +307,41 @@ class ShallowCopyCompilerInvocation : public clang::CompilerInvocation {
   }
 };
 
+// Provides clang AST nodes representing Carbon SemIR entities.
 class CarbonExternalASTSource : public clang::ExternalASTSource {
  public:
   explicit CarbonExternalASTSource(Context* context,
                                    clang::ASTContext* ast_context)
       : context_(context), ast_context_(ast_context) {}
 
+  // Look up decls for `decl_name` inside `decl_context`, adding the decls to
+  // `decl_context`. Returns true if any decls were added.
   auto FindExternalVisibleDeclsByName(
       const clang::DeclContext* decl_context, clang::DeclarationName decl_name,
       const clang::DeclContext* original_decl_context) -> bool override;
 
+  // See clang::ExternalASTSource.
   auto StartTranslationUnit(clang::ASTConsumer* consumer) -> void override;
 
  private:
+  // Map a Carbon entity to a Clang NamedDecl. Returns null if the entity cannot
+  // currently be represented in C++.
+  auto MapInstIdToClangDecl(clang::DeclContext& decl_context,
+                            LookupResult lookup) -> clang::NamedDecl*;
+
   Check::Context* context_;
   clang::ASTContext* ast_context_;
-  clang::NamespaceDecl* carbon_cpp_namespace_ = nullptr;
+  // The association between clang DeclContexts and the corresponding
+  // SemIR::Namespaces in Carbon.
+  // TODO: reuse the SemIR::File::ClangDeclStore to avoid duplicates, and to
+  // enable roundtripping through forward and reverse interop (once we have
+  // syntax/support for that).
+  Map<clang::DeclContext*, SemIR::InstId> scope_map_;
+
+  // Has the "Carbon" C++ namespace been created yet
+  // (this could be replaced with `!scope_map_.empty()` if Carbon::Map supported
+  // `empty()`)
+  bool root_scope_initialized_ = false;
 };
 
 void CarbonExternalASTSource::StartTranslationUnit(
@@ -332,25 +352,28 @@ void CarbonExternalASTSource::StartTranslationUnit(
   translation_unit.setHasExternalVisibleStorage();
 }
 
-// Map a Carbon entity to a Clang NamedDecl.
-static auto MapInstIdToClangDecl(Context& context,
-                                 clang::ASTContext& ast_context,
-                                 clang::DeclContext& decl_context,
-                                 LookupResult lookup) -> clang::NamedDecl* {
+auto CarbonExternalASTSource::MapInstIdToClangDecl(
+    clang::DeclContext& decl_context, LookupResult lookup)
+    -> clang::NamedDecl* {
   auto target_inst_id = lookup.scope_result.target_inst_id();
   if (auto target_inst =
-          context.insts().TryGetAs<SemIR::Namespace>(target_inst_id)) {
-    auto& name_scope = context.name_scopes().Get(target_inst->name_scope_id);
+          context_->insts().TryGetAs<SemIR::Namespace>(target_inst_id)) {
+    auto& name_scope = context_->name_scopes().Get(target_inst->name_scope_id);
     auto* identifier_info =
-        GetClangIdentifierInfo(context, name_scope.name_id());
+        GetClangIdentifierInfo(*context_, name_scope.name_id());
     // TODO: Don't immediately use the decl_context - build any intermediate
     // namespaces iteratively.
     // Eventually add a mapping and use that/populate it/keep it up to date.
     // decl_context could be prepopulated in that mapping and not passed
     // explicitly to MapInstIdToClangDecl.
-    return clang::NamespaceDecl::Create(
-        ast_context, &decl_context, false, clang::SourceLocation(),
+    auto* namespace_decl = clang::NamespaceDecl::Create(
+        *ast_context_, &decl_context, false, clang::SourceLocation(),
         clang::SourceLocation(), identifier_info, nullptr, false);
+    auto result =
+        scope_map_.Insert(namespace_decl->getPrimaryContext(), target_inst_id);
+    CARBON_CHECK(result.is_inserted(), "Inserting over an existing entry.");
+    namespace_decl->setHasExternalVisibleStorage();
+    return namespace_decl;
   }
   return nullptr;
 }
@@ -358,8 +381,12 @@ static auto MapInstIdToClangDecl(Context& context,
 auto CarbonExternalASTSource::FindExternalVisibleDeclsByName(
     const clang::DeclContext* decl_context, clang::DeclarationName decl_name,
     const clang::DeclContext* /*OriginalDC*/) -> bool {
-  if (decl_context != carbon_cpp_namespace_) {
-    if (decl_context->getDeclKind() != clang::Decl::Kind::TranslationUnit) {
+  if (decl_context->getDeclKind() == clang::Decl::Kind::TranslationUnit) {
+    // If the context doesn't already have a mapping between C++ and Carbon,
+    // check if this is the root mapping (for the "Carbon" namespace in the
+    // translation unit scope) and if so, create that mapping.
+
+    if (root_scope_initialized_) {
       return false;
     }
 
@@ -372,17 +399,25 @@ auto CarbonExternalASTSource::FindExternalVisibleDeclsByName(
     // Build the top level 'Carbon' namespace
     auto& ast_context = decl_context->getParentASTContext();
     auto& mutable_tu_decl_context = *ast_context.getTranslationUnitDecl();
-    carbon_cpp_namespace_ = clang::NamespaceDecl::Create(
+    auto* carbon_cpp_namespace = clang::NamespaceDecl::Create(
         ast_context, &mutable_tu_decl_context, false, clang::SourceLocation(),
         clang::SourceLocation(), &ast_context.Idents.get(carbon_namespace_name),
         nullptr, false);
-    carbon_cpp_namespace_->setHasExternalVisibleStorage();
+    carbon_cpp_namespace->setHasExternalVisibleStorage();
+    auto result = scope_map_.Insert(carbon_cpp_namespace->getPrimaryContext(),
+                                    SemIR::Namespace::PackageInstId);
+    CARBON_CHECK(result.is_inserted(), "Inserting over an existing entry.");
     SetExternalVisibleDeclsForName(decl_context, decl_name,
-                                   {carbon_cpp_namespace_});
+                                   {carbon_cpp_namespace});
+    root_scope_initialized_ = true;
     return true;
   }
 
-  // Lookup the name in Carbon package scope
+  auto decl_context_inst_id =
+      scope_map_.Lookup(decl_context->getPrimaryContext());
+  CARBON_CHECK(
+      decl_context_inst_id,
+      "The DeclContext should already be associated with a Carbon InstId.");
 
   llvm::SmallVector<Check::LookupScope> lookup_scopes;
 
@@ -390,7 +425,7 @@ auto CarbonExternalASTSource::FindExternalVisibleDeclsByName(
   // here - completeness should've been checked by clang before this point.
   if (!AppendLookupScopesForConstant(
           *context_, SemIR::LocId::None,
-          context_->constant_values().Get(SemIR::Namespace::PackageInstId),
+          context_->constant_values().Get(decl_context_inst_id.value()),
           SemIR::ConstantId::None, &lookup_scopes)) {
     return false;
   }
@@ -413,8 +448,8 @@ auto CarbonExternalASTSource::FindExternalVisibleDeclsByName(
   }
 
   // Map the found Carbon entity to a Clang NamedDecl.
-  auto* clang_decl = MapInstIdToClangDecl(*context_, *ast_context_,
-                                          *carbon_cpp_namespace_, result);
+  // Use the key to reach the owned, mutable copy of decl_context.
+  auto* clang_decl = MapInstIdToClangDecl(*decl_context_inst_id.key(), result);
   if (!clang_decl) {
     return false;
   }

+ 4 - 15
toolchain/check/testdata/interop/cpp/reverse/simple.carbon

@@ -10,23 +10,10 @@
 // TIP: To dump output, run:
 // TIP:   bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/interop/cpp/reverse/simple.carbon
 
-// --- fail_simple.carbon
-
-library "[[@TEST_NAME]]";
-
-// Demonstrate that the 'Carbon' namespace has been injected into the C++ compilation.
-
-import Cpp inline '''
-// CHECK:STDERR: fail_simple.carbon:[[@LINE+4]]:9: error: no type named 'NonExistent' in namespace 'Carbon' [CppInteropParseError]
-// CHECK:STDERR:    11 | Carbon::NonExistent v;
-// CHECK:STDERR:       | ~~~~~~~~^
-// CHECK:STDERR:
-Carbon::NonExistent v;
-''';
-
-
 // --- other.carbon
 package Other;
+namespace Nested;
+namespace Nested.Again;
 // --- namespace.carbon
 
 library "[[@TEST_NAME]]";
@@ -34,4 +21,6 @@ library "[[@TEST_NAME]]";
 import Other;
 import Cpp inline '''
 namespace X = Carbon::Other;
+namespace Y = Carbon::Other::Nested;
+namespace Z = Carbon::Other::Nested::Again;
 ''';