Skip to content

[flang] Compile-time checking of substring bounds #71453

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
Nov 13, 2023
Merged

Conversation

klausler
Copy link
Contributor

@klausler klausler commented Nov 6, 2023

When the bounds of a substring reference are known during compilation, and are outside the valid range for the character base object, issue an error message.

When the bounds of a substring reference are known during
compilation, and are outside the valid range for the character
base object, issue an error message.
@klausler klausler requested a review from clementval November 6, 2023 22:42
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:semantics labels Nov 6, 2023
@llvmbot
Copy link
Member

llvmbot commented Nov 6, 2023

@llvm/pr-subscribers-flang-semantics

Author: Peter Klausler (klausler)

Changes

When the bounds of a substring reference are known during compilation, and are outside the valid range for the character base object, issue an error message.


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

2 Files Affected:

  • (modified) flang/lib/Semantics/expression.cpp (+24-2)
  • (modified) flang/test/Semantics/data17.f90 (+5-3)
diff --git a/flang/lib/Semantics/expression.cpp b/flang/lib/Semantics/expression.cpp
index 367414a2b4465ce..498a480c5649425 100644
--- a/flang/lib/Semantics/expression.cpp
+++ b/flang/lib/Semantics/expression.cpp
@@ -1059,13 +1059,35 @@ MaybeExpr ExpressionAnalyzer::Analyze(const parser::Substring &ss) {
           const parser::SubstringRange &range{
               std::get<parser::SubstringRange>(ss.t)};
           std::optional<Expr<SubscriptInteger>> first{
-              GetSubstringBound(std::get<0>(range.t))};
+              Fold(GetSubstringBound(std::get<0>(range.t)))};
           std::optional<Expr<SubscriptInteger>> last{
-              GetSubstringBound(std::get<1>(range.t))};
+              Fold(GetSubstringBound(std::get<1>(range.t)))};
           const Symbol &symbol{checked->GetLastSymbol()};
           if (std::optional<DynamicType> dynamicType{
                   DynamicType::From(symbol)}) {
             if (dynamicType->category() == TypeCategory::Character) {
+              auto lbValue{ToInt64(first)};
+              if (!lbValue) {
+                lbValue = 1;
+              }
+              auto ubValue{ToInt64(last)};
+              auto len{dynamicType->knownLength()};
+              if (!ubValue) {
+                ubValue = len;
+              }
+              if (lbValue && ubValue && *lbValue > *ubValue) {
+                // valid, substring is empty
+              } else if (lbValue && *lbValue < 1 && (ubValue || !last)) {
+                Say("Substring must begin at 1 or later, not %jd"_err_en_US,
+                    static_cast<std::intmax_t>(*lbValue));
+                return std::nullopt;
+              } else if (ubValue && len && *ubValue > *len &&
+                  (lbValue || !first)) {
+                Say("Substring must end at %zd or earlier, not %jd"_err_en_US,
+                    static_cast<std::intmax_t>(*len),
+                    static_cast<std::intmax_t>(*ubValue));
+                return std::nullopt;
+              }
               return WrapperHelper<TypeCategory::Character, Designator,
                   Substring>(dynamicType->kind(),
                   Substring{std::move(checked.value()), std::move(first),
diff --git a/flang/test/Semantics/data17.f90 b/flang/test/Semantics/data17.f90
index 8ea9d785a96fb83..86f1c4c6c126950 100644
--- a/flang/test/Semantics/data17.f90
+++ b/flang/test/Semantics/data17.f90
@@ -1,11 +1,13 @@
 ! RUN: %python %S/test_errors.py %s %flang_fc1
-character(4) a, b, c, d, e
+character(4) a, b, c, d, e, f
 !WARNING: DATA statement value '"abcde"' for 'a' has the wrong length
 data a(1:4)/'abcde'/
 !WARNING: DATA statement value '"abc"' for 'b' has the wrong length
 data b(1:4)/'abc'/
 data c/'abcde'/ ! not a substring, conforms
 data d/'abc'/ ! not a substring, conforms
-!ERROR: DATA statement designator 'e(1_8:5_8)' is out of range
-data e(1:5)/'xyz'/
+!ERROR: Substring must end at 4 or earlier, not 5
+data e(1:5)/'abcde'/
+!ERROR: Substring must begin at 1 or later, not 0
+data f(0:4)/'abcde'/
 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 13893a0 into llvm:main Nov 13, 2023
@klausler klausler deleted the b8 branch November 13, 2023 23:03
zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Nov 20, 2023
When the bounds of a substring reference are known during compilation,
and are outside the valid range for the character base object, issue an
error message.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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