Skip to content

Commit

Permalink
Better description of C++ treatment of the non-overridden overloaded …
Browse files Browse the repository at this point in the history
…functions.

Added explicit includes.

PiperOrigin-RevId: 680755865
  • Loading branch information
toli-y authored and Google-ML-Automation committed Oct 3, 2024
1 parent aa382b3 commit d7cb68c
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 33 deletions.
3 changes: 3 additions & 0 deletions xla/hlo/pass/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,14 @@ cc_library(
deps = [
"//xla:status_macros",
"//xla:types",
"//xla:util",
"//xla/hlo/ir:hlo",
"//xla/hlo/ir:hlo_module_group",
"@com_google_absl//absl/container:flat_hash_set",
"@com_google_absl//absl/status",
"@com_google_absl//absl/status:statusor",
"@com_google_absl//absl/strings",
"@tsl//tsl/platform:statusor",
],
)

Expand Down
43 changes: 10 additions & 33 deletions xla/hlo/pass/hlo_pass_interface.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,16 @@ limitations under the License.
#define XLA_HLO_PASS_HLO_PASS_INTERFACE_H_

#include "absl/container/flat_hash_set.h"
#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "absl/strings/string_view.h"
#include "xla/hlo/ir/hlo_computation.h"
#include "xla/hlo/ir/hlo_module.h"
#include "xla/hlo/ir/hlo_module_group.h"
#include "xla/status_macros.h"
#include "xla/types.h"
#include "xla/util.h"
#include "tsl/platform/statusor.h"

namespace xla {

Expand Down Expand Up @@ -66,23 +70,11 @@ class HloPassInterface {

// Run the pass on the given HLO module with specified execution_threads.
// Empty execution_threads list means all execution_threads are included.
// Returns whether it modified the module. Note that due to C++ inheritance
// hides overloaded function, Run(HloModule* module) is not a member function
// of a subclass unless it's explicitly brought to the subclass besides
// implementing the virtual version, for instance,
//
// class MyNewPass : public HloModulePass {
// public:
// MyNewPass();
// absl::string_view name() const override { return "my-new-pass"; }
//
// using HloPassInterface::Run;
// absl::StatusOr<bool> Run(
// HloModule* module,
// const absl::flat_hash_set<absl::string_view>& execution_threads)
// override;
// };
// Returns whether it modified the module.
//
// Note: C++ hides non-explicitly declared overloaded functions.
// You can make all overloaded variants available in the child class by
// adding `using HloPassInterface::Run;` to the child class declaration.
absl::StatusOr<bool> Run(HloModule* module) {
return Run(module, /*execution_threads=*/{});
}
Expand Down Expand Up @@ -115,23 +107,8 @@ class HloPassInterface {
// Ideally, the module group variant would be named "Run" as well, but C++
// does not handle overloaded virtual methods well.
//
// Note that due to C++ inheritance hides overloaded function,
// RunOnModuleGroup(HloModuleGroup* module_group) is not a member function of
// a subclass unless it's explicitly brought to the subclass besides
// implementing the virtual version, for instance,
//
// class MyNewPass : public HloModuleGroupPass {
// public:
// MyNewPass();
// absl::string_view name() const override { return "my-new-pass"; }
//
// using HloPassInterface::RunOnModuleGroup;
// absl::StatusOr<bool> RunOnModuleGroup(
// HloModuleGroup* module_group,
// const absl::flat_hash_set<absl::string_view>& execution_threads)
// override;
// };
//
// See the caveat about C++ hiding overloaded functions in the Run function
// above.
absl::StatusOr<bool> RunOnModuleGroup(HloModuleGroup* module_group) {
return RunOnModuleGroup(module_group, /*execution_threads=*/{});
}
Expand Down

0 comments on commit d7cb68c

Please sign in to comment.