[flang-commits] [flang] [flang][preprocessor] Finesse disabling of function-like macros (PR #71589)

[Peter Klausler via flang-commits]

https://github.com/klausler created https://github.com/llvm/llvm-project/pull/71589

During function-like macro expansion in a standard C/C++ preprocessor, the macro being expanded is disabled from recursive macro expansion. The implementation in this compiler's preprocessor, however, was too broad; the macro expansion needs to be disabled for the "rescanning" phase only, not for actual argument expansion.

(Also corrects an obsolete comment elsewhere that was noticed during reduction of an original test case.)

>From 7dcc29645d57413ad33e87b0f1a896b3ab09ba47 Mon Sep 17 00:00:00 2001
From: Peter Klausler <pklausler at nvidia.com>
Date: Tue, 7 Nov 2023 12:31:47 -0800
Subject: [PATCH] [flang][preprocessor] Finesse disabling of function-like
 macros

During function-like macro expansion in a standard C/C++ preprocessor,
the macro being expanded is disabled from recursive macro expansion.
The implementation in this compiler's preprocessor, however, was too
broad; the macro expansion needs to be disabled for the "rescanning"
phase only, not for actual argument expansion.

(Also corrects an obsolete comment elsewhere that was noticed during
reduction of an original test case.)
---
 flang/lib/Parser/preprocessor.cpp              |  4 ++--
 flang/lib/Semantics/check-call.cpp             | 14 +++++++++-----
 flang/test/Preprocessing/disable-expansion.F90 | 14 ++++++++++++++
 3 files changed, 25 insertions(+), 7 deletions(-)
 create mode 100644 flang/test/Preprocessing/disable-expansion.F90

diff --git a/flang/lib/Parser/preprocessor.cpp b/flang/lib/Parser/preprocessor.cpp
index 5af16323d0129bd..88efcf71445c879 100644
--- a/flang/lib/Parser/preprocessor.cpp
+++ b/flang/lib/Parser/preprocessor.cpp
@@ -397,9 +397,9 @@ std::optional<TokenSequence> Preprocessor::MacroReplacement(
           (n + 1 == argStart.size() ? k : argStart[n + 1] - 1) - at};
       args.emplace_back(TokenSequence(input, at, count));
     }
+    TokenSequence applied{def->Apply(args, prescanner)};
     def->set_isDisabled(true);
-    TokenSequence replaced{
-        ReplaceMacros(def->Apply(args, prescanner), prescanner)};
+    TokenSequence replaced{ReplaceMacros(std::move(applied), prescanner)};
     def->set_isDisabled(false);
     if (!replaced.empty()) {
       ProvenanceRange from{def->replacement().GetProvenanceRange()};
diff --git a/flang/lib/Semantics/check-call.cpp b/flang/lib/Semantics/check-call.cpp
index bf80dbecab009d9..5e0bcf252c59772 100644
--- a/flang/lib/Semantics/check-call.cpp
+++ b/flang/lib/Semantics/check-call.cpp
@@ -1274,11 +1274,15 @@ static void CheckAssociated(evaluate::ActualArguments &arguments,
         return;
       }
       if (const auto &targetArg{arguments[1]}) {
-        // The standard requires that the POINTER= argument be a valid LHS for
-        // a pointer assignment when the TARGET= argument is present.  This,
-        // perhaps unintentionally, excludes function results, including NULL(),
-        // from being used there, as well as INTENT(IN) dummy pointers.
-        // Allow this usage as a benign extension with a portability warning.
+        // The standard requires that the TARGET= argument, when present,
+        // be a valid RHS for a pointer assignment that has the POINTER=
+        // argument as its LHS.  Some popular compilers misinterpret this
+        // requirement more strongly than necessary, and actually validate
+        // the POINTER= argument as if it were serving as the LHS of a pointer
+        // assignment.  This, perhaps unintentionally, excludes function
+        // results, including NULL(), from being used there, as well as
+        // INTENT(IN) dummy pointers.  Detect these conditions and emit
+        // portability warnings.
         if (!evaluate::ExtractDataRef(*pointerExpr) &&
             !evaluate::IsProcedurePointer(*pointerExpr)) {
           context.messages().Say(pointerArg->sourceLocation(),
diff --git a/flang/test/Preprocessing/disable-expansion.F90 b/flang/test/Preprocessing/disable-expansion.F90
new file mode 100644
index 000000000000000..829f33b5016cd67
--- /dev/null
+++ b/flang/test/Preprocessing/disable-expansion.F90
@@ -0,0 +1,14 @@
+! RUN: %flang -E %s | FileCheck %s
+#define KWM a
+#define FLM(x) b FLM2(x) KWM c
+#define FLM2(x) d FLM(x) e
+! CHECK: a
+KWM
+! CHECK: b d flm(y) e a c
+FLM(y)
+! CHECK: b d flm(a) e a c
+FLM(KWM)
+! CHECK: b d flm(b d flm(y) e a c) e a c
+FLM(FLM(y))
+! CHECK: b d flm(b d flm(a) e a c) e a c
+FLM(FLM(KWM))