[rs_bindings_from_cc] allow multiple decl importers to attempt the same decl.
The end goal is to be able to introduce "filters" which prevent an item from being generated by a later importer, because an earlier importer takes "dibs". However, the search before this CL assumed that exactly one importer would attempt to import a decl, and there was no way to make an earlier decl override the results of a later decl.
----
Digging into the meat: the old structure was, IMO, a bit confusing. Look at this loop:
```
std::optional<IR::Item> result;
for (auto& importer : decl_importers_) {
if (importer->CanImport(decl)) {
result = importer->ImportDecl(decl);
}
}
return result;
```
It wasn't clear to me from reading this loop that **exactly one** of these has `CanImport` return true. It is conceivable that it returns true for more than one, and the "last one wins". This can produce fairly nasty bugs if the first one marks it as sucessfully imported, and the second one returns `nullopt` or `UnsupportedItem`. In an earlier CL a few months ago, I suspected exactly this was happening and ended up reproducing this CL just to rule out that flavor of bug.
By making it a short-circuiting search, the described bug is made impossible:
```
for (auto& importer : decl_importers_) {
std::optional<IR::Item> result = importer->ImportDecl(decl);
if (result.has_value()) {
return result;
}
}
return std::nullopt;
```
However, the flip side of this is that now, theoretically, `CanImport` is useless. Returning `false` from `CanImport` is **exactly** equivalent to returning `nullopt` from `ImportDecl`. So we can now delete that function, too, giving us this CL.
After this CL, we can have multiple importers attempt to import a decl, and the behavior if they both attempt it is easy to understand: the first one to return a non-`null` value wins, and no further importers are run. I think this is the obvious behavior, and because only one importer is run, broken side-effects bugs aren't possible in that circumstance.
This should also make importing marginally faster, but who's counting?
PiperOrigin-RevId: 530363429
diff --git a/rs_bindings_from_cc/importer.cc b/rs_bindings_from_cc/importer.cc
index 94fb472..c22d7c9 100644
--- a/rs_bindings_from_cc/importer.cc
+++ b/rs_bindings_from_cc/importer.cc
@@ -549,13 +549,13 @@
std::optional<IR::Item> Importer::ImportDecl(clang::Decl* decl) {
if (IsTransitivelyInPrivate(decl)) return std::nullopt;
- std::optional<IR::Item> result;
for (auto& importer : decl_importers_) {
- if (importer->CanImport(decl)) {
- result = importer->ImportDecl(decl);
+ std::optional<IR::Item> result = importer->ImportDecl(decl);
+ if (result.has_value()) {
+ return result;
}
}
- return result;
+ return std::nullopt;
}
std::optional<IR::Item> Importer::GetImportedItem(const clang::Decl* decl) {
