[Flang] Give a more specific error message for expressions where an IO Unit is expected #126970
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-flang-fir-hlfir @llvm/pr-subscribers-flang-semantics Author: Andre Kuhlenschmidt (akuhlens) ChangesThis PR fixes #125446 by specializing the error message that is generated when an arbitrary expression is used as an IO Unit. See the tests for specific examples, but the general gist is that if you use a non-variable expression as the IO unit argument to a read or write, you now get a more specific error message indicating the the expression could have been a scalar integer expression or character variable. Full diff: https://github.com/llvm/llvm-project/pull/126970.diff 7 Files Affected:
diff --git a/flang/include/flang/Parser/parse-tree.h b/flang/include/flang/Parser/parse-tree.h
index d7b1ae9893c03..f6fb7dfcdb87a 100644
--- a/flang/include/flang/Parser/parse-tree.h
+++ b/flang/include/flang/Parser/parse-tree.h
@@ -2642,7 +2642,7 @@ WRAPPER_CLASS(FileUnitNumber, ScalarIntExpr);
// symbols are known.
struct IoUnit {
UNION_CLASS_BOILERPLATE(IoUnit);
- std::variant<Variable, FileUnitNumber, Star> u;
+ std::variant<Variable, common::Indirection<Expr>, Star> u;
};
// R1206 file-name-expr -> scalar-default-char-expr
diff --git a/flang/lib/Lower/IO.cpp b/flang/lib/Lower/IO.cpp
index 75453721d91a2..515ceb8e89c86 100644
--- a/flang/lib/Lower/IO.cpp
+++ b/flang/lib/Lower/IO.cpp
@@ -1841,7 +1841,9 @@ static mlir::Value genIOUnit(Fortran::lower::AbstractConverter &converter,
int defaultUnitNumber) {
auto &builder = converter.getFirOpBuilder();
if (iounit)
- if (auto *e = std::get_if<Fortran::parser::FileUnitNumber>(&iounit->u))
+ if (auto *e =
+ std::get_if<Fortran::common::Indirection<Fortran::parser::Expr>>(
+ &iounit->u))
return genIOUnitNumber(converter, loc, Fortran::semantics::GetExpr(*e),
ty, csi, stmtCtx);
return builder.create<mlir::arith::ConstantOp>(
diff --git a/flang/lib/Parser/io-parsers.cpp b/flang/lib/Parser/io-parsers.cpp
index 25b09efd40c52..c69ea58738b90 100644
--- a/flang/lib/Parser/io-parsers.cpp
+++ b/flang/lib/Parser/io-parsers.cpp
@@ -23,8 +23,12 @@ namespace Fortran::parser {
// R905 char-variable -> variable
// "char-variable" is attempted first since it's not type constrained but
// syntactically ambiguous with "file-unit-number", which is constrained.
+// Note, "file-unit-number" is replaced by "expr" to allow for better
+// error messages.
TYPE_PARSER(construct<IoUnit>(variable / lookAhead(space / ",);\n"_ch)) ||
- construct<IoUnit>(fileUnitNumber) || construct<IoUnit>(star))
+ construct<IoUnit>(
+ indirect(expr) / (lookAhead(space >> ",)"_ch) || atEndOfStmt)) ||
+ construct<IoUnit>(star))
// R1202 file-unit-number -> scalar-int-expr
TYPE_PARSER(construct<FileUnitNumber>(
diff --git a/flang/lib/Semantics/check-io.cpp b/flang/lib/Semantics/check-io.cpp
index 42c3b9e11efc1..10b32d9af0f88 100644
--- a/flang/lib/Semantics/check-io.cpp
+++ b/flang/lib/Semantics/check-io.cpp
@@ -9,7 +9,9 @@
#include "check-io.h"
#include "definable.h"
#include "flang/Common/format.h"
+#include "flang/Common/indirection.h"
#include "flang/Evaluate/tools.h"
+#include "flang/Parser/characters.h"
#include "flang/Parser/tools.h"
#include "flang/Semantics/expression.h"
#include "flang/Semantics/tools.h"
@@ -576,8 +578,9 @@ void IoChecker::Enter(const parser::IoUnit &spec) {
std::move(mutableVar.u))};
newExpr.source = source;
newExpr.typedExpr = std::move(typedExpr);
- mutableSpec.u = parser::FileUnitNumber{
- parser::ScalarIntExpr{parser::IntExpr{std::move(newExpr)}}};
+ mutableSpec.u = common::Indirection<parser::Expr>{std::move(newExpr)};
+ SetSpecifier(IoSpecKind::Unit);
+ flags_.set(Flag::NumberUnit);
} else if (!dyType || dyType->category() != TypeCategory::Character) {
SetSpecifier(IoSpecKind::Unit);
context_.Say(parser::FindSourceLocation(*var),
@@ -598,6 +601,26 @@ void IoChecker::Enter(const parser::IoUnit &spec) {
} else if (std::get_if<parser::Star>(&spec.u)) {
SetSpecifier(IoSpecKind::Unit);
flags_.set(Flag::StarUnit);
+ } else if (const common::Indirection<parser::Expr> *pexpr{
+ std::get_if<common::Indirection<parser::Expr>>(&spec.u)}) {
+ const auto *expr{GetExpr(context_, *pexpr)};
+ std::optional<evaluate::DynamicType> dyType;
+ if (expr) {
+ dyType = expr->GetType();
+ }
+ if (!expr || !dyType) {
+ context_.Say(parser::FindSourceLocation(*pexpr),
+ "I/O unit must be a character variable or scalar integer expression"_err_en_US);
+ } else if (dyType->category() != TypeCategory::Integer) {
+ context_.Say(parser::FindSourceLocation(*pexpr),
+ "I/O unit must be a character variable or a scalar integer expression, but is an expression of type %s"_err_en_US,
+ parser::ToUpperCaseLetters(dyType->AsFortran()));
+ } else if (expr->Rank() != 0) {
+ context_.Say(parser::FindSourceLocation(*pexpr),
+ "I/O unit number must be scalar"_err_en_US);
+ }
+ SetSpecifier(IoSpecKind::Unit);
+ flags_.set(Flag::NumberUnit);
}
}
diff --git a/flang/test/Semantics/io03.f90 b/flang/test/Semantics/io03.f90
index 6c05924f09dce..3841735ebff95 100644
--- a/flang/test/Semantics/io03.f90
+++ b/flang/test/Semantics/io03.f90
@@ -171,6 +171,22 @@
!ERROR: ID kind (2) is smaller than default INTEGER kind (4)
read(10, id=id2, asynchronous='yes') jj
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type CHARACTER(1)
+ read((msg), *)
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type CHARACTER(KIND=1,LEN=8_8)
+ read("a string", *)
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type CHARACTER(1)
+ read(msg//msg, *)
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type LOGICAL(4)
+ read(.true., *)
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type REAL(4)
+ read(1.0, *)
+ read(internal_fileA, *)
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type CHARACTER(1)
+ read((internal_fileA), *)
+ !ERROR: I/O unit number must be scalar
+ read([1,2,3], *)
+
9 continue
end
diff --git a/flang/test/Semantics/io04.f90 b/flang/test/Semantics/io04.f90
index 7114f14a9488a..4a594bd8b5801 100644
--- a/flang/test/Semantics/io04.f90
+++ b/flang/test/Semantics/io04.f90
@@ -138,6 +138,23 @@
write(*, '(X)')
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type CHARACTER(1)
+ write((msg), *)
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type CHARACTER(KIND=1,LEN=8_8)
+ write("a string", *)
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type CHARACTER(1)
+ write(msg//msg, *)
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type LOGICAL(4)
+ write(.true., *)
+ !ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type REAL(4)
+ write(1.0, *)
+ write(internal_fileA, *)
+ !! Not sure why this isn't an error with this message: I/O unit must be a character variable or a scalar integer expression, but is an expression of type CHARACTER(1)
+ write((internal_fileA), *)
+ !ERROR: I/O unit number must be scalar
+ write([1,2,3], *)
+
+
!ERROR: Output item must not be a procedure
print*, procptr
!ERROR: Output item must not be a procedure
diff --git a/flang/test/Semantics/unsigned-errors.f90 b/flang/test/Semantics/unsigned-errors.f90
index 24d6460bc2fe3..2e2539b40e5ee 100644
--- a/flang/test/Semantics/unsigned-errors.f90
+++ b/flang/test/Semantics/unsigned-errors.f90
@@ -64,7 +64,7 @@
!ERROR: No intrinsic or user-defined ASSIGNMENT(=) matches operand types INTEGER(4) and UNSIGNED(4)
j = 1u
-!ERROR: Must have INTEGER type, but is UNSIGNED(4)
+!ERROR: I/O unit must be a character variable or a scalar integer expression, but is an expression of type UNSIGNED(4)
write(6u,*) 'hi'
!ERROR: ARITHMETIC IF expression must not be an UNSIGNED expression
|
llvmbot
added
flang
akuhlens
force-pushed
the
gh-issue-125446
branch
from
62971e1 to
0cca990
Compare
|
Force pushed to rebase onto main before reviews. |
akuhlens
force-pushed
the
gh-issue-125446
branch
from
620e62c to
5f6c09a
Compare
|
@akuhlens Congratulations on having your first Pull Request (PR) merged into the LLVM Project! Your changes will be combined with recent changes from other authors, then tested by our build bots. If there is a problem with a build, you may receive a report in an email or a comment on this PR. Please check whether problems have been caused by your change specifically, as the builds can include changes from many authors. It is not uncommon for your change to be included in a build that fails due to someone else's changes, or infrastructure issues. How to do this, and the rest of the post-merge process, is covered in detail here. If your change does cause a problem, it may be reverted, or you can revert it yourself. This is a normal part of LLVM development. You can fix your changes and open a new PR to merge them again. If you don't get any reports, no action is required from you. Your changes are working as expected, well done! |
This PR fixes #125446 by specializing the error message that is generated when an arbitrary expression is used as an IO Unit. See the tests for specific examples, but the general gist is that if you use a non-variable expression as the IO unit argument to a read or write, you now get a more specific error message indicating the the expression could have been a scalar integer expression or character variable.