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

[via flang-commits]

llvmbot wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

<details>
<summary>Changes</summary>

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.)

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


3 Files Affected:

- (modified) flang/lib/Parser/preprocessor.cpp (+2-2) 
- (modified) flang/lib/Semantics/check-call.cpp (+9-5) 
- (added) flang/test/Preprocessing/disable-expansion.F90 (+14) 


``````````diff
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))

``````````

</details>


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