From a0a98b19f4308a2ee28698864d9b5dd25dd3092f Mon Sep 17 00:00:00 2001 From: David Wells Date: Fri, 25 Mar 2022 19:50:57 -0400 Subject: [PATCH] Switch to using PIMPL with FunctionParser. --- doc/news/changes/minor/20220325DavidWells | 4 + include/deal.II/base/function_parser.h | 34 +++-- include/deal.II/base/tensor_function_parser.h | 19 +-- source/base/function_parser.cc | 130 ++++++++++-------- source/base/tensor_function_parser.cc | 116 +++++++++------- 5 files changed, 167 insertions(+), 136 deletions(-) create mode 100644 doc/news/changes/minor/20220325DavidWells diff --git a/doc/news/changes/minor/20220325DavidWells b/doc/news/changes/minor/20220325DavidWells new file mode 100644 index 0000000000..b73324794f --- /dev/null +++ b/doc/news/changes/minor/20220325DavidWells @@ -0,0 +1,4 @@ +Fixed: deal.II, compiled with the bundled copy of muParser, can now be +safely linked alongside other projects which use a separate copy of muParser. +
+(David Wells, 2022/03/25) diff --git a/include/deal.II/base/function_parser.h b/include/deal.II/base/function_parser.h index 3e1e28f328..bdac37483b 100644 --- a/include/deal.II/base/function_parser.h +++ b/include/deal.II/base/function_parser.h @@ -29,11 +29,6 @@ #include #include -namespace mu -{ - class Parser; -} - DEAL_II_NAMESPACE_OPEN // Forward declaration @@ -42,6 +37,22 @@ template class Vector; #endif +namespace internal +{ + /** + * deal.II uses muParser as a purely internal dependency. To this end, we do + * not include any muParser headers in this file (and the bundled version of + * the dependency does not install its headers or compile a separate muparser + * library). Hence, to interface with muParser, we use the PIMPL idiom here to + * wrap a pointer to mu::Parser objects. + */ + class muParserBase + { + public: + virtual ~muParserBase() = default; + }; +} // namespace internal + /** * This class implements a function object that gets its value by parsing a * string describing this function. It is a wrapper class for the muparser @@ -259,11 +270,6 @@ public: */ FunctionParser(FunctionParser &&) = delete; - /** - * Destructor. - */ - virtual ~FunctionParser() override; - /** * Copy operator. Objects of this type can not be copied, and * consequently this operator is deleted. @@ -397,11 +403,11 @@ private: mutable Threads::ThreadLocalStorage> vars; /** - * The muParser objects for each thread (and one for each component). We are - * storing a unique_ptr so that we don't need to include the definition of - * mu::Parser in this header. + * The muParser objects (hidden with the PIMPL idiom) for each thread (and one + * for each component). */ - mutable Threads::ThreadLocalStorage>> + mutable Threads::ThreadLocalStorage< + std::vector>> fp; /** diff --git a/include/deal.II/base/tensor_function_parser.h b/include/deal.II/base/tensor_function_parser.h index 2a33e24aee..8e061de926 100644 --- a/include/deal.II/base/tensor_function_parser.h +++ b/include/deal.II/base/tensor_function_parser.h @@ -20,6 +20,7 @@ #include #include +#include #include #include #include @@ -29,11 +30,6 @@ #include #include -namespace mu -{ - class Parser; -} - DEAL_II_NAMESPACE_OPEN // Forward declaration @@ -144,11 +140,6 @@ public: */ TensorFunctionParser(TensorFunctionParser &&) = delete; - /** - * Destructor. - */ - virtual ~TensorFunctionParser() override; - /** * Copy operator. Objects of this type can not be copied, and * consequently this operator is deleted. @@ -278,11 +269,11 @@ private: mutable Threads::ThreadLocalStorage> vars; /** - * The muParser objects for each thread (and one for each component). We are - * storing a unique_ptr so that we don't need to include the definition of - * mu::Parser in this header. + * The muParser objects (hidden with the PIMPL idiom) for each thread (and one + * for each component). */ - mutable Threads::ThreadLocalStorage>> + mutable Threads::ThreadLocalStorage< + std::vector>> tfp; /** diff --git a/source/base/function_parser.cc b/source/base/function_parser.cc index 75505758ca..585b919ebe 100644 --- a/source/base/function_parser.cc +++ b/source/base/function_parser.cc @@ -78,15 +78,6 @@ FunctionParser::FunctionParser(const std::string &expression, } - -// Note: we explicitly define the destructor here (instead of silently using -// the default destructor by declaring nothing in the header) since we do not -// expect muParser.h to be available in user projects: i.e., the destructor -// must be defined in the source file. -template -FunctionParser::~FunctionParser() = default; - - #ifdef DEAL_II_WITH_MUPARSER template @@ -130,6 +121,34 @@ FunctionParser::initialize(const std::string & variables, initialized = true; } +namespace internal +{ + namespace FunctionParserImplementation + { + namespace + { + /** + * PIMPL for mu::Parser. + */ + class Parser : public internal::muParserBase + { + public: + operator mu::Parser &() + { + return parser; + } + + operator const mu::Parser &() const + { + return parser; + } + + protected: + mu::Parser parser; + }; + } // namespace + } // namespace FunctionParserImplementation +} // namespace internal template @@ -147,58 +166,38 @@ FunctionParser::init_muparser() const vars.get().resize(var_names.size()); for (unsigned int component = 0; component < this->n_components; ++component) { - fp.get().emplace_back(new mu::Parser()); + fp.get().emplace_back( + std::make_unique()); + mu::Parser &parser = + dynamic_cast( + *fp.get().back()); for (const auto &constant : constants) { - fp.get()[component]->DefineConst(constant.first, constant.second); + parser.DefineConst(constant.first, constant.second); } for (unsigned int iv = 0; iv < var_names.size(); ++iv) - fp.get()[component]->DefineVar(var_names[iv], &vars.get()[iv]); + parser.DefineVar(var_names[iv], &vars.get()[iv]); // define some compatibility functions: - fp.get()[component]->DefineFun("if", - internal::FunctionParser::mu_if, - true); - fp.get()[component]->DefineOprt("|", internal::FunctionParser::mu_or, 1); - fp.get()[component]->DefineOprt("&", internal::FunctionParser::mu_and, 2); - fp.get()[component]->DefineFun("int", - internal::FunctionParser::mu_int, - true); - fp.get()[component]->DefineFun("ceil", - internal::FunctionParser::mu_ceil, - true); - fp.get()[component]->DefineFun("cot", - internal::FunctionParser::mu_cot, - true); - fp.get()[component]->DefineFun("csc", - internal::FunctionParser::mu_csc, - true); - fp.get()[component]->DefineFun("floor", - internal::FunctionParser::mu_floor, - true); - fp.get()[component]->DefineFun("sec", - internal::FunctionParser::mu_sec, - true); - fp.get()[component]->DefineFun("log", - internal::FunctionParser::mu_log, - true); - fp.get()[component]->DefineFun("pow", - internal::FunctionParser::mu_pow, - true); - fp.get()[component]->DefineFun("erf", - internal::FunctionParser::mu_erf, - true); - fp.get()[component]->DefineFun("erfc", - internal::FunctionParser::mu_erfc, - true); - fp.get()[component]->DefineFun("rand_seed", - internal::FunctionParser::mu_rand_seed, - true); - fp.get()[component]->DefineFun("rand", - internal::FunctionParser::mu_rand, - true); + parser.DefineFun("if", internal::FunctionParser::mu_if, true); + parser.DefineOprt("|", internal::FunctionParser::mu_or, 1); + parser.DefineOprt("&", internal::FunctionParser::mu_and, 2); + parser.DefineFun("int", internal::FunctionParser::mu_int, true); + parser.DefineFun("ceil", internal::FunctionParser::mu_ceil, true); + parser.DefineFun("cot", internal::FunctionParser::mu_cot, true); + parser.DefineFun("csc", internal::FunctionParser::mu_csc, true); + parser.DefineFun("floor", internal::FunctionParser::mu_floor, true); + parser.DefineFun("sec", internal::FunctionParser::mu_sec, true); + parser.DefineFun("log", internal::FunctionParser::mu_log, true); + parser.DefineFun("pow", internal::FunctionParser::mu_pow, true); + parser.DefineFun("erf", internal::FunctionParser::mu_erf, true); + parser.DefineFun("erfc", internal::FunctionParser::mu_erfc, true); + parser.DefineFun("rand_seed", + internal::FunctionParser::mu_rand_seed, + true); + parser.DefineFun("rand", internal::FunctionParser::mu_rand, true); try { @@ -243,7 +242,7 @@ FunctionParser::init_muparser() const } // now use the transformed expression - fp.get()[component]->SetExpr(transformed_expression); + parser.SetExpr(transformed_expression); } catch (mu::ParserError &e) { @@ -293,7 +292,16 @@ FunctionParser::value(const Point & p, try { - return fp.get()[component]->Eval(); + Assert(dynamic_cast( + fp.get()[component].get()), + ExcInternalError()); + // using dynamic_cast in the next line is about 6% slower than + // static_cast, so use the assertion above for debugging and disable + // clang-tidy: + mu::Parser &parser = + static_cast( // NOLINT + *fp.get()[component]); + return parser.Eval(); } catch (mu::ParserError &e) { @@ -329,7 +337,17 @@ FunctionParser::vector_value(const Point &p, vars.get()[dim] = this->get_time(); for (unsigned int component = 0; component < this->n_components; ++component) - values(component) = fp.get()[component]->Eval(); + { + // Same comment in value() applies here too: + Assert(dynamic_cast( + fp.get()[component].get()), + ExcInternalError()); + mu::Parser &parser = + static_cast( // NOLINT + *fp.get()[component]); + + values(component) = parser.Eval(); + } } #else diff --git a/source/base/tensor_function_parser.cc b/source/base/tensor_function_parser.cc index 7f4c6cc1f5..ce062d1bbd 100644 --- a/source/base/tensor_function_parser.cc +++ b/source/base/tensor_function_parser.cc @@ -77,13 +77,6 @@ TensorFunctionParser::TensorFunctionParser( } -// We deliberately delay the definition of the default destructor -// so that we don't need to include the definition of mu::Parser -// in the header file. -template -TensorFunctionParser::~TensorFunctionParser() = default; - - #ifdef DEAL_II_WITH_MUPARSER template @@ -141,6 +134,37 @@ TensorFunctionParser::initialize( +namespace internal +{ + namespace TensorFunctionParserImplementation + { + namespace + { + /** + * PIMPL for mu::Parser. + */ + class Parser : public internal::muParserBase + { + public: + operator mu::Parser &() + { + return parser; + } + + operator const mu::Parser &() const + { + return parser; + } + + protected: + mu::Parser parser; + }; + } // namespace + } // namespace TensorFunctionParserImplementation +} // namespace internal + + + template void TensorFunctionParser::init_muparser() const @@ -156,57 +180,38 @@ TensorFunctionParser::init_muparser() const vars.get().resize(var_names.size()); for (unsigned int component = 0; component < this->n_components; ++component) { - tfp.get().emplace_back(new mu::Parser()); + tfp.get().emplace_back( + std::make_unique< + internal::TensorFunctionParserImplementation::Parser>()); + mu::Parser &parser = + dynamic_cast( + *tfp.get().back()); for (const auto &constant : constants) { - tfp.get()[component]->DefineConst(constant.first, constant.second); + parser.DefineConst(constant.first, constant.second); } for (unsigned int iv = 0; iv < var_names.size(); ++iv) - tfp.get()[component]->DefineVar(var_names[iv], &vars.get()[iv]); + parser.DefineVar(var_names[iv], &vars.get()[iv]); // define some compatibility functions: - tfp.get()[component]->DefineFun("if", - internal::FunctionParser::mu_if, - true); - tfp.get()[component]->DefineOprt("|", internal::FunctionParser::mu_or, 1); - tfp.get()[component]->DefineOprt("&", - internal::FunctionParser::mu_and, - 2); - tfp.get()[component]->DefineFun("int", - internal::FunctionParser::mu_int, - true); - tfp.get()[component]->DefineFun("ceil", - internal::FunctionParser::mu_ceil, - true); - tfp.get()[component]->DefineFun("cot", - internal::FunctionParser::mu_cot, - true); - tfp.get()[component]->DefineFun("csc", - internal::FunctionParser::mu_csc, - true); - tfp.get()[component]->DefineFun("floor", - internal::FunctionParser::mu_floor, - true); - tfp.get()[component]->DefineFun("sec", - internal::FunctionParser::mu_sec, - true); - tfp.get()[component]->DefineFun("log", - internal::FunctionParser::mu_log, - true); - tfp.get()[component]->DefineFun("pow", - internal::FunctionParser::mu_pow, - true); - tfp.get()[component]->DefineFun("erfc", - internal::FunctionParser::mu_erfc, - true); - tfp.get()[component]->DefineFun("rand_seed", - internal::FunctionParser::mu_rand_seed, - true); - tfp.get()[component]->DefineFun("rand", - internal::FunctionParser::mu_rand, - true); + parser.DefineFun("if", internal::FunctionParser::mu_if, true); + parser.DefineOprt("|", internal::FunctionParser::mu_or, 1); + parser.DefineOprt("&", internal::FunctionParser::mu_and, 2); + parser.DefineFun("int", internal::FunctionParser::mu_int, true); + parser.DefineFun("ceil", internal::FunctionParser::mu_ceil, true); + parser.DefineFun("cot", internal::FunctionParser::mu_cot, true); + parser.DefineFun("csc", internal::FunctionParser::mu_csc, true); + parser.DefineFun("floor", internal::FunctionParser::mu_floor, true); + parser.DefineFun("sec", internal::FunctionParser::mu_sec, true); + parser.DefineFun("log", internal::FunctionParser::mu_log, true); + parser.DefineFun("pow", internal::FunctionParser::mu_pow, true); + parser.DefineFun("erfc", internal::FunctionParser::mu_erfc, true); + parser.DefineFun("rand_seed", + internal::FunctionParser::mu_rand_seed, + true); + parser.DefineFun("rand", internal::FunctionParser::mu_rand, true); try { @@ -251,7 +256,7 @@ TensorFunctionParser::init_muparser() const } // now use the transformed expression - tfp.get()[component]->SetExpr(transformed_expression); + parser.SetExpr(transformed_expression); } catch (mu::ParserError &e) { @@ -307,7 +312,14 @@ TensorFunctionParser::value(const Point &p) const for (Number *value_ptr = value.begin_raw(); value_ptr != value.end_raw(); ++value_ptr) { - *value_ptr = tfp.get()[component]->Eval(); + Assert(dynamic_cast< + internal::TensorFunctionParserImplementation::Parser *>( + tfp.get()[component].get()), + ExcInternalError()); + mu::Parser &parser = static_cast< // NOLINT + internal::TensorFunctionParserImplementation::Parser &>( + *tfp.get()[component]); + *value_ptr = parser.Eval(); ++component; } // for } // try -- 2.39.5