rs_bindings_from_cc: Use a custom diagnoser when attempting to instantiate a class template specialization.
So that the diagnostics (for the fallable attempt of instantiation) doesn't gets reported as true failure, which causes Crubit to exit with error, which in turn causes the compilation to fail.
In short, Crubit attempts to instantiate class template specializations more eagerly than header parsing: e.g., `using A = MyTemplate<int>` in a header will cause Crubit to try to instantiate `MyTemplate<int>` while header parsing doesn't attempt to instantiate it. When `MyTemplate<int>` cannot be instantiated, Crubit will error out, while the cc_library target can still be built.
PiperOrigin-RevId: 594233384
Change-Id: Ia1d5fd799a85aebc55cf7c2150729afca4fd969e
diff --git a/bazel/llvm.bzl b/bazel/llvm.bzl
index 27da635..00e0116 100644
--- a/bazel/llvm.bzl
+++ b/bazel/llvm.bzl
@@ -53,7 +53,7 @@
executable = False,
)
-LLVM_COMMIT_SHA = "a4e15416b41459b6f69086a22088520ee826f244"
+LLVM_COMMIT_SHA = "fdb87640ee2be63af9b0e0cd943cb13d79686a03"
def llvm_loader_repository_dependencies():
# This *declares* the dependency, but it won't actually be *downloaded* unless it's used.
diff --git a/rs_bindings_from_cc/importer.cc b/rs_bindings_from_cc/importer.cc
index d77551d..8296788 100644
--- a/rs_bindings_from_cc/importer.cc
+++ b/rs_bindings_from_cc/importer.cc
@@ -46,6 +46,7 @@
#include "clang/AST/Mangle.h"
#include "clang/AST/RawCommentList.h"
#include "clang/AST/Type.h"
+#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/LLVM.h"
#include "clang/Basic/OperatorKinds.h"
@@ -740,8 +741,31 @@
// side-effect and we rely on this here. `decl->getDefinition()` can
// return nullptr before the call to sema and return its definition
// afterwards.
+ // Note: Here we instantiate class template specialization eagerly: its
+ // usages in headers may not require the class template specialization to be
+ // instantiated (and hence it may not be instantiable), but we attempt
+ // instantiation here. So we may attempt non-instantiable template, which
+ // would cause the diagnostic stream to contain error, which would case
+ // clang::tooling::runToolOnCodeWithArgs to return an error status. To avoid
+ // erroring out, we temporarily use our own implementation of
+ // DiagnosticConsumer here.
+ // Unfortunately, this hides template instantiation errors, which sometimes
+ // indicates a malformed header (that should ideally be fixed), which
+ // sometimes indicates Crubit is more eager to instantiate templates than
+ // necessary (e.g., an attempting to instantiate a template that's only
+ // forward-declared).
+ // TODO(b/317866933): Consider surfacing the instantiation error in generated
+ // bindings or to stderr.
+ std::unique_ptr<clang::DiagnosticConsumer> original_client =
+ sema_.getDiagnostics().takeClient();
+ clang::DiagnosticConsumer diagnostic_consumer;
+ sema_.getDiagnostics().setClient(&diagnostic_consumer, false);
+ // Attempt to instantiate.
(void)sema_.isCompleteType(specialization_decl->getLocation(),
ctx_.getRecordType(specialization_decl));
+ // Restore.
+ sema_.getDiagnostics().setClient(original_client.release(),
+ /*ShouldOwnClient=*/true);
// TODO(lukasza): Limit specialization depth? (e.g. using
// `isSpecializationDepthGreaterThan` from earlier prototypes).
diff --git a/rs_bindings_from_cc/test/templates/failed_template_instantiation/BUILD b/rs_bindings_from_cc/test/templates/failed_template_instantiation/BUILD
index 437feb9..6922144 100644
--- a/rs_bindings_from_cc/test/templates/failed_template_instantiation/BUILD
+++ b/rs_bindings_from_cc/test/templates/failed_template_instantiation/BUILD
@@ -6,12 +6,6 @@
crubit_test_cc_library(
name = "failed_template_instantiation",
hdrs = ["failed_template_instantiation.h"],
- tags = [
- # b/316334776: Remove when fixed.
- "manual",
- "nobuilder",
- "notap",
- ],
)
crubit_rust_test(
@@ -20,10 +14,4 @@
cc_deps = [
":failed_template_instantiation",
],
- tags = [
- # b/316334776: Remove when fixed.
- "manual",
- "nobuilder",
- "notap",
- ],
)
diff --git a/rs_bindings_from_cc/test/templates/failed_template_instantiation/failed_template_instantiation.h b/rs_bindings_from_cc/test/templates/failed_template_instantiation/failed_template_instantiation.h
index ad36916..0c76d8b 100644
--- a/rs_bindings_from_cc/test/templates/failed_template_instantiation/failed_template_instantiation.h
+++ b/rs_bindings_from_cc/test/templates/failed_template_instantiation/failed_template_instantiation.h
@@ -5,6 +5,8 @@
#ifndef THIRD_PARTY_CRUBIT_RS_BINDINGS_FROM_CC_TEST_TEMPLATES_FAILED_TEMPLATE_INSTANTIATION_FAILED_TEMPLATE_INSTANTIATION_H_
#define THIRD_PARTY_CRUBIT_RS_BINDINGS_FROM_CC_TEST_TEMPLATES_FAILED_TEMPLATE_INSTANTIATION_FAILED_TEMPLATE_INSTANTIATION_H_
+#pragma clang lifetime_elision
+
template <typename T>
struct UninstantiableTemplate final {
static_assert(false);
@@ -19,9 +21,12 @@
using Ok = InstantiableTemplate<T>;
};
+// This type alias is created just so that we can easily refer to these types in
+// test.rs instead of using their mangled names.
+using Ok = C<int>::Ok;
+using CSpecializedForInt = C<int>;
+
// This is not an error as long as `C<T>::Fail` is not instantiated.
-// However, currently Crubit attempts to instantiate all members of `C<int>` and
-// will error out when the instantiation attempt fails.
inline void Func1(C<int>::Ok){};
inline void Func2(C<int>){};
diff --git a/rs_bindings_from_cc/test/templates/failed_template_instantiation/test.rs b/rs_bindings_from_cc/test/templates/failed_template_instantiation/test.rs
index 107e420..7f5fa6d 100644
--- a/rs_bindings_from_cc/test/templates/failed_template_instantiation/test.rs
+++ b/rs_bindings_from_cc/test/templates/failed_template_instantiation/test.rs
@@ -2,5 +2,12 @@
// Exceptions. See /LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+use failed_template_instantiation::*;
+
#[test]
-fn test_build() {}
+fn test_build() {
+ let ok = Ok::default();
+ Func1(ok);
+ let c = CSpecializedForInt::default();
+ Func2(c);
+}
diff --git a/rs_bindings_from_cc/test/templates/forward_declared_class_template/BUILD b/rs_bindings_from_cc/test/templates/forward_declared_class_template/BUILD
index 916c843..e90b674 100644
--- a/rs_bindings_from_cc/test/templates/forward_declared_class_template/BUILD
+++ b/rs_bindings_from_cc/test/templates/forward_declared_class_template/BUILD
@@ -6,24 +6,12 @@
crubit_test_cc_library(
name = "use_forward_declared_template",
hdrs = ["use_forward_declared_template.h"],
- tags = [
- # b/274663418: Remove when fixed.
- "manual",
- "nobuilder",
- "notap",
- ],
)
crubit_rust_test(
name = "test",
- srcs = ["test.rs"],
+ srcs = ["test_forward_declaration.rs"],
cc_deps = [
":use_forward_declared_template",
],
- tags = [
- # b/274663418: Remove when fixed.
- "manual",
- "nobuilder",
- "notap",
- ],
)
diff --git a/rs_bindings_from_cc/test/templates/forward_declared_class_template/test.rs b/rs_bindings_from_cc/test/templates/forward_declared_class_template/test_forward_declaration.rs
similarity index 100%
rename from rs_bindings_from_cc/test/templates/forward_declared_class_template/test.rs
rename to rs_bindings_from_cc/test/templates/forward_declared_class_template/test_forward_declaration.rs
diff --git a/rs_bindings_from_cc/test/templates/forward_declared_class_template/use_forward_declared_template.h b/rs_bindings_from_cc/test/templates/forward_declared_class_template/use_forward_declared_template.h
index f4bb63f..2d74b73 100644
--- a/rs_bindings_from_cc/test/templates/forward_declared_class_template/use_forward_declared_template.h
+++ b/rs_bindings_from_cc/test/templates/forward_declared_class_template/use_forward_declared_template.h
@@ -5,6 +5,8 @@
#ifndef THIRD_PARTY_CRUBIT_RS_BINDINGS_FROM_CC_TEST_TEMPLATES_FORWARD_DECLARED_CLASS_TEMPLATE_USE_FORWARD_DECLARED_TEMPLATE_H_
#define THIRD_PARTY_CRUBIT_RS_BINDINGS_FROM_CC_TEST_TEMPLATES_FORWARD_DECLARED_CLASS_TEMPLATE_USE_FORWARD_DECLARED_TEMPLATE_H_
+#pragma clang lifetime_elision
+
template <typename T>
struct TemplateWithFullDefinition final {
// We need a field to depend on T to introduce the "implicit instantiation of
