[flang] Fix copy elision assumption.

[Meinersbur]

Summary

Like D88794, the Restorer depends 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.

Inhibit recreating the restorer object completely to avoid the implicit creation of a second restorer object that would try to move the same object. The factory class ScopedSet is changed to use unnamed return value optimization which is mandatory in C++17.

[klausler]

A much smaller work-around: extend the sole constructor of Restorer to accept a reference and a value, and then modify ScopedSet to construct its result as part of the return statement. Restorer doesn't need move construction or move assignment for its clients.

[Meinersbur]

Implement @klausler's suggestion.

[klausler]

flang/include/flang/Common/restorer.h

25

Here, and below: we're using lower-case names in flang/ for variables.

flang/include/flang/Common/restorer.h

 // // x is back to 3
 
 #ifndef FORTRAN_COMMON_RESTORER_H_
 #define FORTRAN_COMMON_RESTORER_H_
 #include "idioms.h"
 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_;
 };
 
 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_

flang/lib/Semantics/check-declarations.cpp

if (const auto *overriddenBinding{
           "A type-bound procedure binding may not have the same name as a parent component"_err_en_US);
     }
   }
   CheckPassArg(symbol, &binding.symbol(), binding);
 }
 
 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;
   }
   if (scope.IsParameterizedDerivedTypeInstantiation()) {
     auto restorer{common::ScopedSet(scopeIsUninstantiatedPDT_, false)};
     auto restorer2{context_.foldingContext().messages().SetContext(
         scope.instantiationContext().get())};
     for (const auto &pair : scope) {