Related : #587
ONE/compiler/luci/pass/src/CircleOptimizer.cpp
class OptimizeOptionsImpl : public luci::CircleOptimizer::Options
{
public:
void enable(Algorithm) final;
bool query(Algorithm) final;
private:
std::vector<Algorithm> _algorithms;
};
void OptimizeOptionsImpl::enable(Algorithm algo) { _algorithms.push_back(algo); }
bool OptimizeOptionsImpl::query(Algorithm algo)
{
std::vector<Algorithm>::iterator it = std::find(_algorithms.begin(), _algorithms.end(), algo);
if (it == _algorithms.end())
return false;
return true;
}
```cxx
class CircleOptimizer final
{
public:
struct Options
{
enum Algorithm
{
FuseInstanceNorm,
ResolveCustomOpBatchMatMul
};
virtual void enable(Algorithm) = 0;
virtual bool query(Algorithm) = 0;
};
public:
// TODO maybe caller can provide Options as ctor parameters
Options *options(void);
public:
void optimize(loco::Graph *) const;
private:
std::unique_ptr
};
Class `OptimizeOptionsImpl` has a compiler-generated destructor. It is non-empty because of its field `_algorithms`. A pointer to class `OptimizeOptionsImpl` is upcast to class `luci::CircleOptimizer::Options` which doesn't have a virtual destructor.
So, I will be changed to like below.
```cxx
struct Options
{
enum Algorithm
{
FuseInstanceNorm,
ResolveCustomOpBatchMatMul
};
virtual ~Options() = default; // note here
virtual void enable(Algorithm) = 0;
virtual bool query(Algorithm) = 0;
};
If you have a better idea, please let me know.
adding virtual ~Options() = default; seems OK to me :)
IMHO Option class SHOULD have a virtual destructor ;-D
If Option has no virtual destructor, the destructor of OptimizeOptionsImpl is not invoked when _option field is destructed.
(Optional) It would be better to mark OptimizeOptionsImpl as final ;-)
class OptimizeOptionsImpl final : public luci::CircleOptimizer::Options
Most helpful comment
IMHO
Optionclass SHOULD have a virtual destructor ;-DIf
Optionhas no virtual destructor, the destructor ofOptimizeOptionsImplis not invoked when_optionfield is destructed.(Optional) It would be better to mark
OptimizeOptionsImplas final ;-)