Author: Michael Kruse Date: 2020-12-14T14:07:05-06:00 New Revision: 2aa43358060c6b34fb9cdc6c4321e958f62331e7
URL: https://github.com/llvm/llvm-project/commit/2aa43358060c6b34fb9cdc6c4321e958f62331e7 DIFF: https://github.com/llvm/llvm-project/commit/2aa43358060c6b34fb9cdc6c4321e958f62331e7.diff LOG: [flang] Fix copy elision assumption. Before this patch, the Restorer depended on copy elision to happen. Without copy elision, the function ScopedSet calls the move constructor before its dtor. The dtor will prematurely restore the reference to the original value. Instead of relying the compiler to not use the Restorer's copy constructor, delete its copy and assign operators. Hence, callers cannot move or copy a Restorer object anymore, and have to explicitly provide the reset state. ScopedSet avoids calling move/copy operations by relying on unnamed return value optimization, which is mandatory in C++17. Reviewed By: klausler Differential Revision: https://reviews.llvm.org/D88797 Added: Modified: flang/include/flang/Common/restorer.h flang/lib/Semantics/check-declarations.cpp Removed: ################################################################################ diff --git a/flang/include/flang/Common/restorer.h b/flang/include/flang/Common/restorer.h index 47e54237fe43..4d5f5e4e2c81 100644 --- a/flang/include/flang/Common/restorer.h +++ b/flang/include/flang/Common/restorer.h @@ -22,9 +22,16 @@ namespace Fortran::common { template <typename A> class Restorer { public: - explicit Restorer(A &p) : p_{p}, original_{std::move(p)} {} + explicit Restorer(A &p, A original) : p_{p}, original_{std::move(original)} {} ~Restorer() { p_ = std::move(original_); } + // Inhibit any recreation of this restorer that would result in two restorers + // trying to restore the same reference. + Restorer(const Restorer &) = delete; + Restorer(Restorer &&that) = delete; + const Restorer &operator=(const Restorer &) = delete; + const Restorer &operator=(Restorer &&that) = delete; + private: A &p_; A original_; @@ -32,15 +39,15 @@ template <typename A> class Restorer { template <typename A, typename B> common::IfNoLvalue<Restorer<A>, B> ScopedSet(A &to, B &&from) { - Restorer<A> result{to}; + A original{std::move(to)}; to = std::move(from); - return result; + return Restorer<A>{to, std::move(original)}; } template <typename A, typename B> common::IfNoLvalue<Restorer<A>, B> ScopedSet(A &to, const B &from) { - Restorer<A> result{to}; + A original{std::move(to)}; to = from; - return result; + return Restorer<A>{to, std::move(original)}; } } // namespace Fortran::common #endif // FORTRAN_COMMON_RESTORER_H_ diff --git a/flang/lib/Semantics/check-declarations.cpp b/flang/lib/Semantics/check-declarations.cpp index 0d2e2e86241c..dd76f6710070 100644 --- a/flang/lib/Semantics/check-declarations.cpp +++ b/flang/lib/Semantics/check-declarations.cpp @@ -1498,7 +1498,7 @@ void CheckHelper::CheckProcBinding( void CheckHelper::Check(const Scope &scope) { scope_ = &scope; - common::Restorer<const Symbol *> restorer{innermostSymbol_}; + common::Restorer<const Symbol *> restorer{innermostSymbol_, innermostSymbol_}; if (const Symbol * symbol{scope.symbol()}) { innermostSymbol_ = symbol; } _______________________________________________ llvm-branch-commits mailing list llvm-branch-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
