Fix b/333737712: incorrect namespace assignment for items in reopened namespaces.
This ended up being a pretty simple problem after all. We do have independent namespace decl IDs for the original vs reopened namespace, so there _is_ an item ID that the nested declaration could be assigned as a parent -- it just was assigned the wrong one. Why? Well, the previous version of the code used `getEnclosingNamespaceContext`, which loops over the parents until it finds the namespace, and then returns the _canonicalized_ namespace decl: the _first_ namespace decl with that name, not the actual direct parent, which might be a redeclaration. The fix, then, is to not use `getEnclosingNamespaceContext`, but to reproduce the same loop in our code.
PiperOrigin-RevId: 655052095
Change-Id: Ic2b8c85f65a8bdb6f4de55f61231bc5f3ef49e47
diff --git a/rs_bindings_from_cc/importer.cc b/rs_bindings_from_cc/importer.cc
index 635c638..0e76854 100644
--- a/rs_bindings_from_cc/importer.cc
+++ b/rs_bindings_from_cc/importer.cc
@@ -394,27 +394,30 @@
absl::StatusOr<std::optional<ItemId>> Importer::GetEnclosingItemId(
clang::Decl* decl) {
- clang::DeclContext* decl_context = decl->getDeclContext();
-
- // Class template specializations are always emitted in the top-level
- // namespace. See also Importer::GetOrderedItemIdsOfTemplateInstantiations.
- if (clang::isa<clang::ClassTemplateSpecializationDecl>(decl))
- return std::nullopt;
-
- if (decl_context->isFunctionOrMethod()) {
- return std::nullopt;
- }
- if (auto* record_decl = clang::dyn_cast<clang::RecordDecl>(decl_context)) {
- if (!EnsureSuccessfullyImported(record_decl)) {
- return absl::InvalidArgumentError("Couldn't import the parent");
+ for (clang::DeclContext* decl_context = decl->getDeclContext();;
+ decl_context = decl_context->getParent()) {
+ if (decl_context->isTranslationUnit()) {
+ return std::nullopt;
}
- return GenerateItemId(record_decl);
- }
- auto enclosing_namespace = decl_context->getEnclosingNamespaceContext();
- if (enclosing_namespace->isTranslationUnit()) return std::nullopt;
+ // Class template specializations are always emitted in the top-level
+ // namespace. See also Importer::GetOrderedItemIdsOfTemplateInstantiations.
+ if (clang::isa<clang::ClassTemplateSpecializationDecl>(decl))
+ return std::nullopt;
- auto namespace_decl = clang::cast<clang::NamespaceDecl>(enclosing_namespace);
- return GenerateItemId(namespace_decl);
+ if (decl_context->isFunctionOrMethod()) {
+ return std::nullopt;
+ }
+ if (auto* record_decl = clang::dyn_cast<clang::RecordDecl>(decl_context)) {
+ if (!EnsureSuccessfullyImported(record_decl)) {
+ return absl::InvalidArgumentError("Couldn't import the parent");
+ }
+ return GenerateItemId(record_decl);
+ }
+ if (auto* namespace_decl =
+ clang::dyn_cast<clang::NamespaceDecl>(decl_context)) {
+ return GenerateItemId(namespace_decl);
+ }
+ }
}
std::vector<ItemId> Importer::GetItemIdsInSourceOrder(
diff --git a/rs_bindings_from_cc/test/namespace/inline/BUILD b/rs_bindings_from_cc/test/namespace/inline/BUILD
index 7dfb53b..9b17c03 100644
--- a/rs_bindings_from_cc/test/namespace/inline/BUILD
+++ b/rs_bindings_from_cc/test/namespace/inline/BUILD
@@ -1,5 +1,3 @@
-"""End-to-end test of inheritance."""
-
load("//common:crubit_wrapper_macros_oss.bzl", "crubit_rust_test")
load("//rs_bindings_from_cc/test:test_bindings.bzl", "crubit_test_cc_library")
diff --git a/rs_bindings_from_cc/test/namespace/reopened/BUILD b/rs_bindings_from_cc/test/namespace/reopened/BUILD
new file mode 100644
index 0000000..a530287
--- /dev/null
+++ b/rs_bindings_from_cc/test/namespace/reopened/BUILD
@@ -0,0 +1,26 @@
+load("//common:crubit_wrapper_macros_oss.bzl", "crubit_rust_test")
+load("//rs_bindings_from_cc/test:test_bindings.bzl", "crubit_test_cc_library")
+
+package(default_applicable_licenses = ["//:license"])
+
+crubit_test_cc_library(
+ name = "reopened_namespace",
+ hdrs = ["reopened_namespace.h"],
+ deps = [":original_namespace"],
+)
+
+crubit_test_cc_library(
+ name = "original_namespace",
+ hdrs = ["original_namespace.h"],
+ # disable Crubit
+ aspect_hints = [],
+)
+
+crubit_rust_test(
+ name = "test",
+ srcs = ["test.rs"],
+ cc_deps = [":reopened_namespace"],
+ proc_macro_deps = [
+ "//common:item_exists",
+ ],
+)
diff --git a/rs_bindings_from_cc/test/namespace/reopened/original_namespace.h b/rs_bindings_from_cc/test/namespace/reopened/original_namespace.h
new file mode 100644
index 0000000..ed32993
--- /dev/null
+++ b/rs_bindings_from_cc/test/namespace/reopened/original_namespace.h
@@ -0,0 +1,15 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef THIRD_PARTY_CRUBIT_RS_BINDINGS_FROM_CC_TEST_NAMESPACE_INLINE_ORIGINAL_NAMESPACE_H_
+#define THIRD_PARTY_CRUBIT_RS_BINDINGS_FROM_CC_TEST_NAMESPACE_INLINE_ORIGINAL_NAMESPACE_H_
+
+// Namespaces can be opened in one header with Crubit disabled,
+// and then _reopened_ in another header with Crubit enabled.
+// See b/333737712
+namespace foo {
+struct SomeStruct {};
+} // namespace foo
+
+#endif // THIRD_PARTY_CRUBIT_RS_BINDINGS_FROM_CC_TEST_NAMESPACE_INLINE_ORIGINAL_NAMESPACE_H_
diff --git a/rs_bindings_from_cc/test/namespace/reopened/reopened_namespace.h b/rs_bindings_from_cc/test/namespace/reopened/reopened_namespace.h
new file mode 100644
index 0000000..98ed379
--- /dev/null
+++ b/rs_bindings_from_cc/test/namespace/reopened/reopened_namespace.h
@@ -0,0 +1,18 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+#ifndef CRUBIT_RS_BINDINGS_FROM_CC_TEST_NAMESPACE_INLINE_INLINE_H_
+#define CRUBIT_RS_BINDINGS_FROM_CC_TEST_NAMESPACE_INLINE_INLINE_H_
+
+#include "rs_bindings_from_cc/test/namespace/reopened/original_namespace.h"
+
+namespace foo {
+
+inline SomeStruct FunctionUsesNamespaceType() { return SomeStruct(); }
+
+inline int Returns42() { return 42; }
+
+} // namespace foo
+
+#endif // CRUBIT_RS_BINDINGS_FROM_CC_TEST_NAMESPACE_INLINE_INLINE_H_
diff --git a/rs_bindings_from_cc/test/namespace/reopened/test.rs b/rs_bindings_from_cc/test/namespace/reopened/test.rs
new file mode 100644
index 0000000..2b23d66
--- /dev/null
+++ b/rs_bindings_from_cc/test/namespace/reopened/test.rs
@@ -0,0 +1,16 @@
+// Part of the Crubit project, under the Apache License v2.0 with LLVM
+// Exceptions. See /LICENSE for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+
+use item_exists::value_exists;
+use reopened_namespace::foo;
+
+#[test]
+fn test_not_present() {
+ assert!(!value_exists!(foo::FunctionUsesNamespaceType));
+}
+
+#[test]
+fn test_reopened_namespace() {
+ assert_eq!(42, foo::Returns42());
+}
