[flang][Parser] Convert applyMem to invoke non-void members

[kparzysz]

Currently applyMem(f, a, ...) calls a.f(...), but still returns the result of parser a. This is in contrast to applyFunction(f, a, ...), which returns the result of calling f(a, ...).

The use case for this is being able to parse quoted or unquoted strings, and store the result as std::string:

construct<IdentifierOrString>(
  (space >> charLiteralConstantWithoutKind) ||
  applyMem(&Name::ToString, Parser<Name>{})) // Parser<Name>{}.ToString()

The applyMem combinator is currently unused.

[llvmbot]

@llvm/pr-subscribers-flang-parser

Author: Krzysztof Parzyszek (kparzysz)

Changes

Currently applyMem(f, a, ...) calls a.f(...), but still returns the result of parser a. This is in contrast to applyFunction(f, a, ...), which returns the result of calling f(a, ...).

The use case for this is being able to parse quoted or unquoted strings, and store the result as std::string:

construct<IdentifierOrString>(
  (space >> charLiteralConstantWithoutKind) ||
  applyMem(&Name::ToString, Parser<Name>{})) // Parser<Name>{}.ToString()

The applyMem combinator is currently unused.

---

Full diff: https://github.com/llvm/llvm-project/pull/119782.diff

2 Files Affected:

• (modified) flang/docs/ParserCombinators.md (+1-1)
• (modified) flang/lib/Parser/basic-parsers.h (+23-31)

diff --git a/flang/docs/ParserCombinators.md b/flang/docs/ParserCombinators.md
index 7cb77deba21971..076e76f703c49c 100644
--- a/flang/docs/ParserCombinators.md
+++ b/flang/docs/ParserCombinators.md
@@ -141,7 +141,7 @@ collect the values that they return.
 * `applyLambda([](&&x){}, p1, p2, ...)` is the same thing, but for lambdas
   and other function objects.
 * `applyMem(mf, p1, p2, ...)` is the same thing, but invokes a member
-  function of the result of the first parser for updates in place.
+  function of the result of the first parser.
 
 ### Token Parsers
 Last, we have these basic parsers on which the actual grammar of the Fortran
diff --git a/flang/lib/Parser/basic-parsers.h b/flang/lib/Parser/basic-parsers.h
index 515b5993d67376..1a8c14e7048f64 100644
--- a/flang/lib/Parser/basic-parsers.h
+++ b/flang/lib/Parser/basic-parsers.h
@@ -580,11 +580,11 @@ template <typename PA> inline constexpr auto defaulted(PA p) {
 // applyLambda(f, ...) is the same concept extended to std::function<> functors.
 // It is not constexpr.
 //
-// Member function application is supported by applyMem(f, a).  If the
-// parser a succeeds and returns some value ax, the result is that returned
-// by ax.f().  Additional parser arguments can be specified to supply their
-// results to the member function call, so applyMem(f, a, b) succeeds if
-// both a and b do so and returns the result of calling ax.f(std::move(bx)).
+// Member function application is supported by applyMem(&C::f, a).  If the
+// parser a succeeds and returns some value ax of type C, the result is that
+// returned by ax.f().  Additional parser arguments can be specified to supply
+// their results to the member function call, so applyMem(&C::f, a, b) succeeds
+// if both a and b do so and returns the result of calling ax.f(std::move(bx)).
 
 // Runs a sequence of parsers until one fails or all have succeeded.
 // Collects their results in a std::tuple<std::optional<>...>.
@@ -654,39 +654,31 @@ inline /* not constexpr */ auto applyLambda(
 }
 
 // Member function application
-template <typename OBJPARSER, typename... PARSER> class AMFPHelper {
-  using resultType = typename OBJPARSER::resultType;
-
-public:
-  using type = void (resultType::*)(typename PARSER::resultType &&...);
-};
-template <typename OBJPARSER, typename... PARSER>
-using ApplicableMemberFunctionPointer =
-    typename AMFPHelper<OBJPARSER, PARSER...>::type;
-
-template <typename OBJPARSER, typename... PARSER, std::size_t... J>
-inline auto ApplyHelperMember(
-    ApplicableMemberFunctionPointer<OBJPARSER, PARSER...> mfp,
-    ApplyArgs<OBJPARSER, PARSER...> &&args, std::index_sequence<J...>) ->
-    typename OBJPARSER::resultType {
-  ((*std::get<0>(args)).*mfp)(std::move(*std::get<J + 1>(args))...);
-  return std::get<0>(std::move(args));
+template <typename MEMFUNC, typename OBJPARSER, typename... PARSER,
+    std::size_t... J>
+inline auto ApplyHelperMember(MEMFUNC mfp,
+    ApplyArgs<OBJPARSER, PARSER...> &&args, std::index_sequence<J...>) {
+  return ((*std::get<0>(args)).*mfp)(std::move(*std::get<J + 1>(args))...);
 }
 
-template <typename OBJPARSER, typename... PARSER> class ApplyMemberFunction {
-  using funcType = ApplicableMemberFunctionPointer<OBJPARSER, PARSER...>;
+template <typename MEMFUNC, typename OBJPARSER, typename... PARSER>
+class ApplyMemberFunction {
+  static_assert(std::is_member_function_pointer_v<MEMFUNC>);
+  using funcType = MEMFUNC;
 
 public:
-  using resultType = typename OBJPARSER::resultType;
+  using resultType =
+      std::invoke_result_t<MEMFUNC, typename OBJPARSER::resultType, PARSER...>;
+
   constexpr ApplyMemberFunction(const ApplyMemberFunction &) = default;
-  constexpr ApplyMemberFunction(funcType f, OBJPARSER o, PARSER... p)
+  constexpr ApplyMemberFunction(MEMFUNC f, OBJPARSER o, PARSER... p)
       : function_{f}, parsers_{o, p...} {}
   std::optional<resultType> Parse(ParseState &state) const {
     ApplyArgs<OBJPARSER, PARSER...> results;
     using Sequence1 = std::index_sequence_for<OBJPARSER, PARSER...>;
     using Sequence2 = std::index_sequence_for<PARSER...>;
     if (ApplyHelperArgs(parsers_, results, state, Sequence1{})) {
-      return ApplyHelperMember<OBJPARSER, PARSER...>(
+      return ApplyHelperMember<MEMFUNC, OBJPARSER, PARSER...>(
           function_, std::move(results), Sequence2{});
     } else {
       return std::nullopt;
@@ -698,11 +690,11 @@ template <typename OBJPARSER, typename... PARSER> class ApplyMemberFunction {
   const std::tuple<OBJPARSER, PARSER...> parsers_;
 };
 
-template <typename OBJPARSER, typename... PARSER>
+template <typename MEMFUNC, typename OBJPARSER, typename... PARSER>
 inline constexpr auto applyMem(
-    ApplicableMemberFunctionPointer<OBJPARSER, PARSER...> mfp,
-    const OBJPARSER &objParser, PARSER... parser) {
-  return ApplyMemberFunction<OBJPARSER, PARSER...>{mfp, objParser, parser...};
+    MEMFUNC memfn, const OBJPARSER &objParser, PARSER... parser) {
+  return ApplyMemberFunction<MEMFUNC, OBJPARSER, PARSER...>{
+      memfn, objParser, parser...};
 }
 
 // As is done with function application via applyFunction() above, class

[klausler]

Please add a use of the new capabilities in the parser or some kind of test case.

[klausler]

Does the new version no longer work for updates in place?

[kparzysz]

It returns the value returned by the member call, so for member functions returning void (as it was before) it wouldn't work. For member functions returning reference to *this or something equivalent it should be fine.

Is modifying in-place is still needed? If so I'd need to separate the code from this PR into its own parser.

[klausler]

You'll need to look at usage sites (of which there are not many) to be sure this comment is obsolete. I wouldn't have added that comment unless I had been thinking about mutation in situ at the time.

[kparzysz]

I couldn't find any uses of this in the upstream sources. I thought that maybe it's used in some downstream project.

[klausler]

Go for it, then. Downstream usage can adapt if needed.

[kparzysz]

Please add a use of the new capabilities in the parser or some kind of test case.

Will do.

PS. Technically I'm on vacation for the end of the year, so my replies or updates may be sparser than usual.

[kparzysz]

Decided against it in the end. There would only be a single use case for this at the moment, and it will likely go away, since that code may need updates in the future.