[StringFormat] Throw Errors for mismatch between format and argument #2633

gptsarthak · 2023-10-15T18:09:43Z

I thought of trying to implement some basic error handling in the runtime library for StringFormat.
It uses the basic fmt generated by the compute_fmt_specifier_and_arg to record the sequence and type of each argument passed, and matches it with the format given as input.
I just use perror() and then exit(1) to handle the errors. Let me know if there is a better way to do this.

program example
    implicit none
    integer :: x = 134
    print "(i3)", x
    print "(a)", x
    print "(i5)", "Hello World"
end program example

(base) ➜  lfortran git:(fmt2) ✗ lfortran integration_tests/format_10.f90 
134
Type mismatch in print/write statment: expected string, got integer
: Success

integration_tests/CMakeLists.txt

src/libasr/codegen/asr_to_llvm.cpp

certik

I think that this is fine, thanks!

gptsarthak · 2023-10-15T19:53:31Z

Test write_03.f90 is failing:

SUBROUTINE DVOUT()
   CHARACTER*80       LINE
9999 FORMAT( / 1X, A, / 1X, A )
   LOUT = 10
   WRITE( LOUT, FMT = 9999 )IFMT, LINE( 1: LLL )
END SUBROUTINE

PROGRAM WRITE_03
   CALL DVOUT()
END PROGRAM

I dont know if it should or should not fail. We dont have enough information. In main, it prints out empty newline.
If we do WRITE (*,*) IFMT, LINE( 1: LLL ), the output is 0.

Edit: I set the FAIL flag on the test for now.

certik · 2023-10-17T15:30:23Z

That test is indeed incorrect. It was introduced in #1909 to test a workaround (that you since fixed) for using a format using a label.

Let me submit a fixed test, just a minute.

certik · 2023-10-17T15:44:04Z

Here is the fixed test: #2647. Once that PR is merged, go ahead and rebase and get everything passing, it should work now.

certik · 2023-10-19T13:31:54Z

integration_tests/CMakeLists.txt

@@ -1007,7 +1008,7 @@ RUN(NAME implicit_deallocate_01 LABELS gfortran llvm)

 RUN(NAME write_01 LABELS gfortran llvm)
 RUN(NAME write_02 LABELS gfortran llvm)
-RUN(NAME write_03 LABELS gfortran llvmImplicit NOFAST)
+RUN(NAME write_03 FAIL LABELS gfortran llvmImplicit NOFAST)


Suggested change

RUN(NAME write_03 FAIL LABELS gfortran llvmImplicit NOFAST)

RUN(NAME write_03 LABELS gfortran llvmImplicit NOFAST)

certik · 2023-10-19T13:32:13Z

src/libasr/runtime/lfortran_intrinsics.c

+void fmterror(const char* fmttype, const char* argtype) {
+    char error[100];
+    sprintf(error, "Type mismatch in print or write statement: expected %s, got %s\n", fmttype, argtype);
+    perror(error);


Don't use perror, just print and exit.

certik · 2023-10-19T13:32:55Z

tests/print4.f90

@@ -0,0 +1,6 @@
+program example


Suggested change

program example

program print4

and I would move this file to tests/errors/

certik · 2023-10-19T13:33:36Z

Otherwise it's good. Also rebase on top of the latest main.

certik · 2023-10-19T15:15:00Z

tests/print4.f90

+    integer :: x = 134
+    print "(a)", x
+    print "(i5)", "Hello World"
+end program


move this to tests/error

I observed that if the file location is not in the tests directory itself we get a runtime_error: raw_fd_ostream failed error.

Ok, that's fine for now then, I made this #2783 to be investigated and fixed later.

certik · 2023-10-19T15:15:06Z

examples/temp.f90

+    double precision :: s
+    s = 23.5678_8
+    print '(D15.8,1X)',s
+end program


remove this

certik · 2023-11-02T17:24:44Z

I rebased it just in case.

certik

Thanks!

certik · 2023-11-02T19:35:01Z

@gptsarthak unfortunately even though the CI passes, it fails on Apple M1. I had to revert it in #2786 with more information in there. I created an issue #2787 to fix this. Do you think you would have time to investigate?

gptsarthak · 2023-11-02T19:38:57Z

Valgrind does not show any memory leaks right now. I will investigate it properly tomorrow.

==13368== Memcheck, a memory error detector
==13368== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==13368== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==13368== Command: ./format_04.out
==13368== 
ok b
okb
Success!
          Hello World!
danc  in themoon
ab  cdef  ghi  jkl  qwerty 12
123     00456
012345  ***
  0.12D+03  -0.123457D+03  0.12D+02  *******
 -1.23D+02   12.34568D+01 -1.23D+01   1.23D+02
 -0.01E+04   0.001235E+05 -1.23E+01   1.23E+02
  1  2  3  4  5  6  7  8  9 10 11 12
  1  2  3  4 hello
  5  6  7  8 hello
  9 10 11 12 hello
 13 14
12345
-12345
0.123456001D+3 0.1D+3 0.12D+2
-0.123456001D+3 -0.1D+3 -0.12D+2
Hello
 12345678.000 23.567800 .35
-12345678.000 ********* -.35
 2.000000E+00
 0.000000E+00
   0.0000000000000000000012345
x:1.12 y:4.5E+00
x:  9.99E-01
x:  1.00E-01
==13368== 
==13368== HEAP SUMMARY:
==13368==     in use at exit: 24 bytes in 2 blocks
==13368==   total heap usage: 706 allocs, 704 frees, 15,995 bytes allocated
==13368== 
==13368== LEAK SUMMARY:
==13368==    definitely lost: 0 bytes in 0 blocks
==13368==    indirectly lost: 0 bytes in 0 blocks
==13368==      possibly lost: 0 bytes in 0 blocks
==13368==    still reachable: 24 bytes in 2 blocks
==13368==         suppressed: 0 bytes in 0 blocks
==13368== Reachable blocks (those to which a pointer was found) are not shown.
==13368== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==13368== 
==13368== For lists of detected and suppressed errors, rerun with: -s
==13368== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

gptsarthak force-pushed the fmt2 branch 2 times, most recently from 017903f to 0a4450c Compare October 15, 2023 19:11

certik reviewed Oct 15, 2023

View reviewed changes

integration_tests/CMakeLists.txt Outdated Show resolved Hide resolved

certik reviewed Oct 15, 2023

View reviewed changes

src/libasr/codegen/asr_to_llvm.cpp Outdated Show resolved Hide resolved

certik approved these changes Oct 15, 2023

View reviewed changes

gptsarthak force-pushed the fmt2 branch from 0a4450c to 1cca247 Compare October 15, 2023 19:36

gptsarthak force-pushed the fmt2 branch 2 times, most recently from 4c568f9 to bac70b2 Compare October 15, 2023 20:22

gptsarthak mentioned this pull request Oct 17, 2023

string formatting segfault #2646

Open

certik reviewed Oct 19, 2023

View reviewed changes

gptsarthak force-pushed the fmt2 branch 2 times, most recently from 2404dbd to 8af9d31 Compare October 19, 2023 14:18

certik reviewed Oct 19, 2023

View reviewed changes

examples/temp.f90 Outdated

double precision :: s

s = 23.5678_8

print '(D15.8,1X)',s

end program

Copy link

Contributor

certik Oct 19, 2023

Choose a reason for hiding this comment

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

remove this

certik marked this pull request as draft October 19, 2023 15:15

gptsarthak force-pushed the fmt2 branch from 8af9d31 to 46f7029 Compare November 2, 2023 15:35

gptsarthak marked this pull request as ready for review November 2, 2023 17:11

Throw Errors for mismatch between format and argument

a179dd3

certik force-pushed the fmt2 branch from 46f7029 to a179dd3 Compare November 2, 2023 17:24

certik enabled auto-merge November 2, 2023 17:24

certik approved these changes Nov 2, 2023

View reviewed changes

certik merged commit cdf1028 into lfortran:main Nov 2, 2023
20 checks passed

certik mentioned this pull request Nov 2, 2023

Revert "[StringFormat] Throw Errors for mismatch between format and argument" #2786

Merged

certik mentioned this pull request Nov 2, 2023

Throw Errors for mismatch between format and argument #2787

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[StringFormat] Throw Errors for mismatch between format and argument #2633

[StringFormat] Throw Errors for mismatch between format and argument #2633

gptsarthak commented Oct 15, 2023

certik left a comment

gptsarthak commented Oct 15, 2023 •

edited

certik commented Oct 17, 2023

certik commented Oct 17, 2023

certik Oct 19, 2023

certik Oct 19, 2023

certik Oct 19, 2023

certik commented Oct 19, 2023

certik Oct 19, 2023

gptsarthak Nov 2, 2023

certik Nov 2, 2023 •

edited

certik Oct 19, 2023

certik commented Nov 2, 2023

certik left a comment

certik commented Nov 2, 2023 •

edited

gptsarthak commented Nov 2, 2023

	RUN(NAME write_03 FAIL LABELS gfortran llvmImplicit NOFAST)
	RUN(NAME write_03 LABELS gfortran llvmImplicit NOFAST)

[StringFormat] Throw Errors for mismatch between format and argument #2633

[StringFormat] Throw Errors for mismatch between format and argument #2633

Conversation

gptsarthak commented Oct 15, 2023

certik left a comment

Choose a reason for hiding this comment

gptsarthak commented Oct 15, 2023 • edited

certik commented Oct 17, 2023

certik commented Oct 17, 2023

certik Oct 19, 2023

Choose a reason for hiding this comment

certik Oct 19, 2023

Choose a reason for hiding this comment

certik Oct 19, 2023

Choose a reason for hiding this comment

certik commented Oct 19, 2023

certik Oct 19, 2023

Choose a reason for hiding this comment

gptsarthak Nov 2, 2023

Choose a reason for hiding this comment

certik Nov 2, 2023 • edited

Choose a reason for hiding this comment

certik Oct 19, 2023

Choose a reason for hiding this comment

certik commented Nov 2, 2023

certik left a comment

Choose a reason for hiding this comment

certik commented Nov 2, 2023 • edited

gptsarthak commented Nov 2, 2023

gptsarthak commented Oct 15, 2023 •

edited

certik Nov 2, 2023 •

edited

certik commented Nov 2, 2023 •

edited