This is an archive of the discontinued LLVM Phabricator instance.

Differential D84504

[flang]Verify C7107, C7108, C7109 from - Clause 7 constraint checks for f18.
ClosedPublic

Authored by sameeranjoshi on Jul 24 2020, 3:13 AM.

Details

Reviewers
PeteSteinfeld
kiranktp
anchu-rajendran
klausler
jdoerfert
DavidTruby
Commits
rG8830f1170dae: [flang]Verify C7107, C7108, C7109 from - Clause 7 constraint checks for f18.
Summary
  1. Annotate the sources with constraint numbers.
  2. Add tests for

    *C7107 (R765) digit shall have one of the values 0 or 1. *C7108 (R766) digit shall have one of the values 0 through 7. *C7109 (R764) A boz-literal-constant shall appear only as a data-stmt-constant in a DATA statement, or where explicitly allowed in 16.9 as an actual argument of an intrinsic procedure.

Diff Detail

Event Timeline

sameeranjoshi created this revision.Jul 24 2020, 3:13 AM
Herald added a reviewer: jdoerfert. · View Herald TranscriptJul 24 2020, 3:13 AM
Herald added a reviewer: DavidTruby. · View Herald Transcript
sameeranjoshi marked an inline comment as done.Jul 24 2020, 4:20 AM
sameeranjoshi added inline comments.
flang/test/Semantics/boz-literal-constants.f90
35

Seems like this 8.6.7(11) was missing a test.
Current llvm-trunk flang outputs below error:

"DATA statement value could not be converted to the type 'COMPLEX(4)' of the object 'rescmplx'"

Is the error (with this patch) expected to be printed here(i.e is that getting ignored due to above error)?

49

Is error below a more proper one?

Typeless (BOZ) not allowed for both 'i=' & 'j=' arguments.

16.9.65(3)

J shall be of type integer or a boz-literal-constant. If both I and J are of type integer, they shall have the same kind type parameter. I and J shall not both be boz-literal-constants

restricts variables I and J when both are boz-literal-constants, whereas the current error with flang trunk assumes 'i' to be wrong, and which might not give proper information to the user.

PeteSteinfeld requested changes to this revision.Jul 24 2020, 7:18 AM

Thanks for working on this!

Please update the messages and the test results. Also, please add a reference to C7109 in the source code where that constraint is enforced.

flang/test/Semantics/boz-literal-constants.f90
35

I suspect that the error message produced in this instance changed when I committed the fix for D83917. In that change, I give the normally typeless BOZ literal constants the type INTEGER. The new error message seems OK to me, but you should update the test results.

49

I would say:

Typeless (BOZ) not allowed for either 'i=' & 'j=' arguments.
This revision now requires changes to proceed.Jul 24 2020, 7:18 AM
sameeranjoshi updated this revision to Diff 280522.Jul 24 2020, 11:04 AM
sameeranjoshi edited the summary of this revision. (Show Details)

Address review comments.

Herald added a project: Restricted Project. · View Herald TranscriptJul 24 2020, 11:04 AM
sameeranjoshi updated this revision to Diff 280523.Jul 24 2020, 11:11 AM

Add missing constraint number in sources.

sameeranjoshi marked 3 inline comments as done.Jul 24 2020, 11:12 AM
PeteSteinfeld requested changes to this revision.Jul 24 2020, 11:49 AM
PeteSteinfeld added inline comments.
flang/lib/Evaluate/intrinsics.cpp
1134 ↗(On Diff #280523)

Sorry for not noticing this earlier, but the message should say:

Typeless (BOZ) not allowed for either '%s=' or '%s=' arguments
flang/test/Semantics/boz-literal-constants.f90
50

The message also needs to be corrected here.

This revision now requires changes to proceed.Jul 24 2020, 11:49 AM
Harbormaster completed remote builds in B65602: Diff 280523.Jul 24 2020, 11:58 AM
Harbormaster completed remote builds in B65601: Diff 280522.
tskeith added a subscriber: tskeith.Jul 24 2020, 12:40 PM
tskeith added inline comments.
flang/test/Semantics/boz-literal-constants.f90
49

I think @sameeranjoshi's version of this message is right. They can't both be BOZ literals but either one can be.

PeteSteinfeld added inline comments.Jul 24 2020, 12:47 PM
flang/test/Semantics/boz-literal-constants.f90
49

Good catch, Tim. I just re-read the standard, and you're correct.

sameeranjoshi updated this revision to Diff 280947.Jul 27 2020, 9:42 AM
sameeranjoshi marked 4 inline comments as done.

Revert to older message.

sameeranjoshi edited the summary of this revision. (Show Details)Jul 27 2020, 9:44 AM
Harbormaster completed remote builds in B65851: Diff 280947.Jul 27 2020, 10:47 AM
PeteSteinfeld accepted this revision.Jul 27 2020, 1:07 PM

All builds, tests, and looks good!

This revision is now accepted and ready to land.Jul 27 2020, 1:07 PM
sameeranjoshi updated this revision to Diff 282274.Jul 31 2020, 11:14 AM

Changes in commit message for using arc.

This revision was landed with ongoing or failed builds.Jul 31 2020, 11:15 AM
Closed by commit rG8830f1170dae: [flang]Verify C7107, C7108, C7109 from - Clause 7 constraint checks for f18. (authored by sameeranjoshi). · Explain Why
This revision was automatically updated to reflect the committed changes.
Sameeran joshi <joshisameeran17@gmail.com> added a commit: rG8830f1170dae: [flang]Verify C7107, C7108, C7109 from - Clause 7 constraint checks for f18..
Harbormaster completed remote builds in B66589: Diff 282274.Jul 31 2020, 12:16 PM

Revision Contents

PathSize
flang/
lib/
Semantics/
3 lines
test/
Semantics/
79 lines

Diff 280380

flang/lib/Semantics/expression.cpp

Show First 20 Lines • Show All 627 Lines • ▼ Show 20 Lines case 'x':
break; break;
default: default:
CRASH_NO_CASE; CRASH_NO_CASE;
} }
CHECK(*p == '"'); CHECK(*p == '"');
++p; ++p;
auto value{BOZLiteralConstant::Read(p, base, false /*unsigned*/)}; auto value{BOZLiteralConstant::Read(p, base, false /*unsigned*/)};
if (*p != '"') { if (*p != '"') {
Say("Invalid digit ('%c') in BOZ literal '%s'"_err_en_US, *p, x.v); Say("Invalid digit ('%c') in BOZ literal '%s'"_err_en_US, *p,
x.v); // C7107, C7108
return std::nullopt; return std::nullopt;
} }
if (value.overflow) { if (value.overflow) {
Say("BOZ literal '%s' too large"_err_en_US, x.v); Say("BOZ literal '%s' too large"_err_en_US, x.v);
return std::nullopt; return std::nullopt;
} }
return AsGenericExpr(std::move(value.value)); return AsGenericExpr(std::move(value.value));
} }
▲ Show 20 Lines • Show All 2,508 Lines • Show Last 20 Lines

flang/test/Semantics/boz-literal-constants.f90

  • This file was added.
! RUN: %S/test_errors.sh %s %t %f18
! Confirm enforcement of constraints and restrictions in 7.7
! C7107, C7108, C7109
subroutine bozchecks
! Type declaration statements
integer :: f, realpart = B"0101", img = B"1111", resint
logical :: resbit
complex :: rescmplx
real :: dbl, e
! C7107
!ERROR: Invalid digit ('a') in BOZ literal 'b"110a"'
integer, parameter :: a = B"110A"
!ERROR: Invalid digit ('2') in BOZ literal 'b"1232"'
integer, parameter :: b = B"1232"
!ERROR: BOZ literal 'b"010101010101010101010101011111111111111111111111111111111111111111111111111111111111111111111111111111111111000000000000000000000000000000000000"' too large
integer, parameter :: b1 = B"010101010101010101010101011111111111111111111&
&111111111111111111111111111111111111111111111&
&111111111111111111000000000000000000000000000&
&000000000"
! C7108
!ERROR: Invalid digit ('8') in BOZ literal 'o"8"'
integer :: c = O"8"
!ERROR: Invalid digit ('a') in BOZ literal 'o"a"'
integer :: d = O"A"
! C7109
! A) can appear only in data statement
! B) Argument to intrinsics listed from 16.9 below
! BGE, BGT, BLE, BLT, CMPLX, DBLE, DSHIFTL,
! DSHIFTR, IAND, IEOR, INT, IOR, MERGE_BITS, REAL
! part A
data f / Z"AA" / ! OK
! ERROR: BOZ literal should appear in a DATA statement only as a value for an integer object, but 'rescmplx' is 'COMPLEX(4)'"
sameeranjoshiAuthorUnsubmitted
Done
ReplyInline Actions

Seems like this 8.6.7(11) was missing a test.
Current llvm-trunk flang outputs below error:

"DATA statement value could not be converted to the type 'COMPLEX(4)' of the object 'rescmplx'"

Is the error (with this patch) expected to be printed here(i.e is that getting ignored due to above error)?

sameeranjoshi: Seems like this `8.6.7(11)` was missing a test. Current llvm-trunk flang outputs below error…
PeteSteinfeldUnsubmitted
Done
ReplyInline Actions

I suspect that the error message produced in this instance changed when I committed the fix for D83917. In that change, I give the normally typeless BOZ literal constants the type INTEGER. The new error message seems OK to me, but you should update the test results.

PeteSteinfeld: I suspect that the error message produced in this instance changed when I committed the fix for…
data rescmplx / B"010101" /
! part B
resbit = BGE(B"0101", B"1111")
resbit = BGT(Z"0101", B"1111")
resbit = BLE(B"0101", B"1111")
resbit = BLT(B"0101", B"1111")
res = CMPLX (realpart, img, 4)
res = CMPLX (B"0101", B"1111", 4)
dbl = DBLE(B"1111")
dbl = DBLE(realpart)
!ERROR: Typeless (BOZ) not allowed for 'i=' argument
sameeranjoshiAuthorUnsubmitted
Done
ReplyInline Actions

Is error below a more proper one?

Typeless (BOZ) not allowed for both 'i=' & 'j=' arguments.

16.9.65(3)

J shall be of type integer or a boz-literal-constant. If both I and J are of type integer, they shall have the same kind type parameter. I and J shall not both be boz-literal-constants

restricts variables I and J when both are boz-literal-constants, whereas the current error with flang trunk assumes 'i' to be wrong, and which might not give proper information to the user.

sameeranjoshi: Is error below a more proper one? ``` Typeless (BOZ) not allowed for both 'i=' & 'j=' arguments.
PeteSteinfeldUnsubmitted
Done
ReplyInline Actions

I would say:

Typeless (BOZ) not allowed for either 'i=' & 'j=' arguments.
PeteSteinfeld: I would say: ``` Typeless (BOZ) not allowed for either 'i=' & 'j=' arguments. ```
tskeithUnsubmitted
Done
ReplyInline Actions

I think @sameeranjoshi's version of this message is right. They can't both be BOZ literals but either one can be.

tskeith: I think @sameeranjoshi's version of this message is right. They can't both be BOZ literals but…
PeteSteinfeldUnsubmitted
Done
ReplyInline Actions

Good catch, Tim. I just re-read the standard, and you're correct.

PeteSteinfeld: Good catch, Tim. I just re-read the standard, and you're correct.
dbl = DSHIFTL(B"0101",B"0101",2)
PeteSteinfeldUnsubmitted
Done
ReplyInline Actions

The message also needs to be corrected here.

PeteSteinfeld: The message also needs to be corrected here.
!ERROR: Typeless (BOZ) not allowed for 'i=' argument
dbl = DSHIFTR(B"1010",B"1010",2)
dbl = DSHIFTL(B"0101",5,2) ! OK
dbl = DSHIFTR(B"1010",5,2) ! OK
!ERROR: Typeless (BOZ) not allowed for 'i=' argument
resint = IAND(B"0001", B"0011")
resint = IAND(B"0001", 3)
!ERROR: Typeless (BOZ) not allowed for 'i=' argument
resint = IEOR(B"0001", B"0011")
resint = IEOR(B"0001", 3)
resint = INT(B"1010")
!ERROR: Typeless (BOZ) not allowed for 'i=' argument
res = IOR(B"0101", B"0011")
res = IOR(B"0101", 3)
res = MERGE_BITS(13,3,11)
res = MERGE_BITS(B"1101",3,11)
!ERROR: Typeless (BOZ) not allowed for 'i=' argument
res = MERGE_BITS(B"1101",B"0011",11)
!ERROR: Typeless (BOZ) not allowed for 'i=' argument
res = MERGE_BITS(B"1101",B"0011",B"1011")
res = MERGE_BITS(B"1101",3,B"1011")
res = REAL(B"1101")
end subroutine