Skip to content

[flang] Adjust "doubled operator" expression extension #93353

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

Merged
merged 1 commit into from
Jun 3, 2024

Conversation

klausler
Copy link
Contributor

Most Fortran compilers accept "doubled operators" as a language extension. This is the use of a unary '+' or '-' operator that is not the first unparenthesized operator in an expression, as in 'x*-y'.

This compiler has implemented this extension, but in a way that's different from other compilers' behavior. I interpreted the unary '+'/'-' as a unary operator in the sense of C/C++, giving it a higher priority than any binary (dyadic) operator.

All other compilers with this extension, however, give a unary '+'/'-' a lower precedence than exponentiation (''), a binary operator that C/C++ lacks. And this interpretation makes more sense for Fortran, anyway, where the standard conforming '-xy' must mean '-(x**y)' already.

This patch makes 'x*-yz' parse as 'x*-(yz)', not 'x*(-y)**z)', and adds a test to ensure that it does.

@klausler klausler requested review from clementval and vdonaldson May 24, 2024 23:05
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics flang:parser labels May 24, 2024
Most Fortran compilers accept "doubled operators" as a language extension.
This is the use of a unary '+' or '-' operator that is not the
first unparenthesized operator in an expression, as in 'x*-y'.

This compiler has implemented this extension, but in a way that's
different from other compilers' behavior.  I interpreted the
unary '+'/'-' as a unary operator in the sense of C/C++, giving
it a higher priority than any binary (dyadic) operator.

All other compilers with this extension, however, give a
unary '+'/'-' a lower precedence than exponentiation ('**'),
a binary operator that C/C++ lacks.  And this interpretation
makes more sense for Fortran, anyway, where the standard conforming
'-x**y' must mean '-(x**y)' already.

This patch makes 'x*-y**z' parse as 'x*-(y**z)', not 'x*(-y)**z)',
and adds a test to ensure that it does.
@llvmbot
Copy link
Member

llvmbot commented May 24, 2024

@llvm/pr-subscribers-flang-parser

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

Most Fortran compilers accept "doubled operators" as a language extension. This is the use of a unary '+' or '-' operator that is not the first unparenthesized operator in an expression, as in 'x*-y'.

This compiler has implemented this extension, but in a way that's different from other compilers' behavior. I interpreted the unary '+'/'-' as a unary operator in the sense of C/C++, giving it a higher priority than any binary (dyadic) operator.

All other compilers with this extension, however, give a unary '+'/'-' a lower precedence than exponentiation (''), a binary operator that C/C++ lacks. And this interpretation makes more sense for Fortran, anyway, where the standard conforming '-xy' must mean '-(x**y)' already.

This patch makes 'x*-yz' parse as 'x*-(yz)', not 'x*(-y)**z)', and adds a test to ensure that it does.


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

4 Files Affected:

  • (modified) flang/docs/Extensions.md (+2)
  • (modified) flang/include/flang/Common/Fortran-features.h (+1-1)
  • (modified) flang/lib/Parser/expr-parsers.cpp (+15-9)
  • (added) flang/test/Evaluate/signed-mult-opd.f90 (+12)
diff --git a/flang/docs/Extensions.md b/flang/docs/Extensions.md
index 7b872c786c82c..27d007f3a88d4 100644
--- a/flang/docs/Extensions.md
+++ b/flang/docs/Extensions.md
@@ -356,6 +356,8 @@ end
 * A derived type that meets (most of) the requirements of an interoperable
   derived type can be used as such where an interoperable type is
   required, with warnings, even if it lacks the BIND(C) attribute.
+* A "mult-operand" in an expression can be preceded by a unary
+  `+` or `-` operator.
 
 ### Extensions supported when enabled by options
 
diff --git a/flang/include/flang/Common/Fortran-features.h b/flang/include/flang/Common/Fortran-features.h
index 15c4af63f4be7..b3635f2e8f6ae 100644
--- a/flang/include/flang/Common/Fortran-features.h
+++ b/flang/include/flang/Common/Fortran-features.h
@@ -24,7 +24,7 @@ ENUM_CLASS(LanguageFeature, BackslashEscapes, OldDebugLines,
     DoubleComplex, Byte, StarKind, ExponentMatchingKindParam, QuadPrecision,
     SlashInitialization, TripletInArrayConstructor, MissingColons,
     SignedComplexLiteral, OldStyleParameter, ComplexConstructor, PercentLOC,
-    SignedPrimary, FileName, Carriagecontrol, Convert, Dispose,
+    SignedMultOperand, FileName, Carriagecontrol, Convert, Dispose,
     IOListLeadingComma, AbbreviatedEditDescriptor, ProgramParentheses,
     PercentRefAndVal, OmitFunctionDummies, CrayPointer, Hollerith, ArithmeticIF,
     Assign, AssignedGOTO, Pause, OpenACC, OpenMP, CUDA, CruftAfterAmpersand,
diff --git a/flang/lib/Parser/expr-parsers.cpp b/flang/lib/Parser/expr-parsers.cpp
index b27366d02308e..a47aae166b575 100644
--- a/flang/lib/Parser/expr-parsers.cpp
+++ b/flang/lib/Parser/expr-parsers.cpp
@@ -87,14 +87,8 @@ constexpr auto primary{instrumented("primary"_en_US,
 // R1002 level-1-expr -> [defined-unary-op] primary
 // TODO: Reasonable extension: permit multiple defined-unary-ops
 constexpr auto level1Expr{sourced(
-    first(primary, // must come before define op to resolve .TRUE._8 ambiguity
-        construct<Expr>(construct<Expr::DefinedUnary>(definedOpName, primary)),
-        extension<LanguageFeature::SignedPrimary>(
-            "nonstandard usage: signed primary"_port_en_US,
-            construct<Expr>(construct<Expr::UnaryPlus>("+" >> primary))),
-        extension<LanguageFeature::SignedPrimary>(
-            "nonstandard usage: signed primary"_port_en_US,
-            construct<Expr>(construct<Expr::Negate>("-" >> primary)))))};
+    primary || // must come before define op to resolve .TRUE._8 ambiguity
+    construct<Expr>(construct<Expr::DefinedUnary>(definedOpName, primary)))};
 
 // R1004 mult-operand -> level-1-expr [power-op mult-operand]
 // R1007 power-op -> **
@@ -105,7 +99,19 @@ struct MultOperand {
   static inline std::optional<Expr> Parse(ParseState &);
 };
 
-static constexpr auto multOperand{sourced(MultOperand{})};
+// Extension: allow + or - before a mult-operand
+// Such a unary operand has lower precedence than exponentiation,
+// so -x**2 is -(x**2), not (-x)**2; this matches all other
+// compilers with this extension.
+static constexpr auto standardMultOperand{sourced(MultOperand{})};
+static constexpr auto multOperand{standardMultOperand ||
+    extension<LanguageFeature::SignedMultOperand>(
+        "nonstandard usage: signed mult-operand"_port_en_US,
+        construct<Expr>(
+            construct<Expr::UnaryPlus>("+" >> standardMultOperand))) ||
+    extension<LanguageFeature::SignedMultOperand>(
+        "nonstandard usage: signed mult-operand"_port_en_US,
+        construct<Expr>(construct<Expr::Negate>("-" >> standardMultOperand)))};
 
 inline std::optional<Expr> MultOperand::Parse(ParseState &state) {
   std::optional<Expr> result{level1Expr.Parse(state)};
diff --git a/flang/test/Evaluate/signed-mult-opd.f90 b/flang/test/Evaluate/signed-mult-opd.f90
new file mode 100644
index 0000000000000..75b533de80e3b
--- /dev/null
+++ b/flang/test/Evaluate/signed-mult-opd.f90
@@ -0,0 +1,12 @@
+! RUN: %python %S/test_folding.py %s %flang_fc1
+module m
+  integer, parameter :: j = 2
+  ! standard cases
+  logical, parameter :: test_1 = -j**2 == -4
+  logical, parameter :: test_2 = 4-j**2 == 0
+  ! extension cases
+  logical, parameter :: test_3 = 4+-j**2 == 0 ! not 8
+  logical, parameter :: test_4 = 2*-j**2 == -8 ! not 8
+  logical, parameter :: test_5 = -j**2+-j**2 == -8 ! not 8
+  logical, parameter :: test_6 = j**2*-j**2 == -16 ! not 16
+end

Copy link
Contributor

@clementval clementval left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@klausler klausler merged commit 68f4e46 into llvm:main Jun 3, 2024
8 checks passed
@klausler klausler deleted the precedence branch June 3, 2024 18:58
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.

3 participants