[flang] Fix non-deterministic line output function

[ijan1]

Summary

The evaluation order for the | operator is undefined
(in contrast to the short-circuiting || operator). The arguments are
stored in variables to force a specific evaluation order.

A test in D107575 relies on this change.

[kiranchandramohan]

LGTM. Please wait for approval from @klausler or @Meinersbur.

flang/lib/Semantics/check-declarations.cpp

1155

Nit: You probably do not need "&" in the capture list since you have explicitly captured the variables (opName, specific, proc) required.

[klausler]

flang/lib/Semantics/check-declarations.cpp

1156

We use modern C++ braced initialization in the front-end, as a rule, for better error detection.

You don't need parentheses on a return statement.

[Meinersbur]

flang/lib/Semantics/check-declarations.cpp

1155

specific and proc should be captured by-reference, at they are in the function arguments.

Even better, I'd convert all captures into lambda arguments, such that one can see at the call site what data the check depends on.

[Meinersbur]

flang/lib/Semantics/check-declarations.cpp

1158

With both argument having been evaluated, it is now possible to use the logical or (both arguments are bool).

[ijan1]

Updating to use braced initialisation, removed parentheses in the return statement and converted the captures into lambda parameters.

[klausler]

Please use braced initialization for the lambda as well.

[Meinersbur]

flang/lib/Semantics/check-declarations.cpp

1156

Nothing is implicitly captured.

[ijan1]

Using braced initialisation for the lambda.

[klausler]

Looks great to me now; thank you for the patch.

[ijan1]

Commenting so that my inline comment shows.

flang/lib/Semantics/check-declarations.cpp

1156

I thought I had left a comment but it seems like it was unsubmitted. The this pointer is being captured here.

[Meinersbur]

flang/lib/Semantics/check-declarations.cpp

1156

OK. Could have used [this], it is always captured by value even with [&]. No need to change after it having been committed.

flang/lib/Semantics/check-declarations.cpp

 //===-- lib/Semantics/check-declarations.cpp ------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 // Static declaration checking

 
 // Check if this procedure can be used for defined operators (see 15.4.3.4.2).
 bool CheckHelper::CheckDefinedOperator(SourceName opName, GenericKind kind,
     const Symbol &specific, const Procedure &proc) {
   if (context_.HasError(specific)) {
     return false;
   }
   std::optional<parser::MessageFixedText> msg;
+  auto checkDefinedOperatorArgs = 
+      [&](SourceName opName, const Symbol &specific, const Procedure &proc) {
+    bool arg0Defined {CheckDefinedOperatorArg(opName, specific, proc, 0)};
+    bool arg1Defined {CheckDefinedOperatorArg(opName, specific, proc, 1)};
+    return arg0Defined && arg1Defined;
+  };
   if (specific.attrs().test(Attr::NOPASS)) { // C774
     msg = "%s procedure '%s' may not have NOPASS attribute"_err_en_US;
   } else if (!proc.functionResult.has_value()) {
     msg = "%s procedure '%s' must be a function"_err_en_US;
   } else if (proc.functionResult->IsAssumedLengthCharacter()) {
     msg = "%s function '%s' may not have assumed-length CHARACTER(*)"
           " result"_err_en_US;
   } else if (auto m{CheckNumberOfArgs(kind, proc.dummyArguments.size())}) {
     msg = std::move(m);
-  } else if (!CheckDefinedOperatorArg(opName, specific, proc, 0) |
-      !CheckDefinedOperatorArg(opName, specific, proc, 1)) {
+  } else if (!checkDefinedOperatorArgs(opName, specific, proc)) {
     return false; // error was reported
   } else if (ConflictsWithIntrinsicOperator(kind, proc)) {
     msg = "%s function '%s' conflicts with intrinsic operator"_err_en_US;
   } else {
     return true; // OK
   }
   SayWithDeclaration(
       specific, std::move(*msg), MakeOpName(opName), specific.name());