Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang][preprocessor] Finesse disabling of function-like macros #71589

Merged
merged 1 commit into from
Nov 13, 2023

Conversation

klausler
Copy link
Contributor

@klausler klausler commented Nov 7, 2023

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

@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics flang:parser labels Nov 7, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Nov 7, 2023

@llvm/pr-subscribers-flang-parser

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

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

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.)
@klausler klausler merged commit f2bf44b into llvm:main Nov 13, 2023
3 checks passed
@klausler klausler deleted the steve3 branch November 13, 2023 23:25
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
…#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.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
flang:parser flang:semantics flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants