flang-compiler / f18

F18 is a front-end for Fortran intended to replace the existing front-end in the Flang compiler
230 stars 48 forks source link

Semantic checks for 5 data constraints: C874, C875, C878, C880, C881 #1081

Closed anchuraj closed 4 years ago

anchuraj commented 4 years ago

This commit has implementation of semantic checks for 5 constraints on data object. 3 more constraint-checks need to be implemented.

anchuraj commented 4 years ago

Hi, @klausler , @tskeith, I would like to get clarifications on two constraints

  1. C878 (R841) The array-element shall be a variable. To throw the error, the only test case I could think of is the case where array element is a constant. So my code checks if data object is a constant and if yes, it throws the error. Please correct me if it has to be implemented in a different way.
  2. C879 (R841) The scalar-structure-component shall be a variable. According to my understanding, the code for C878 should apply for C879 as well (as it checks whether the DataRef is a constant or not). I tried writing a testcase where a scalar component of a struture is a constant with the snippet below: type newNum integer a end type type(newNum), parameter :: num In this case, I see that num%a is not identified as a constant expression by F18. Shouldn't it be identified? If so, it requires modification in the implementation of IsConstantExpr Please correct me if I am interpreting things wrong. Thank you ,
klausler commented 4 years ago

Hi, @klausler , @tskeith, I would like to get clarifications on two constraints

  1. C878 (R841) The array-element shall be a variable. To throw the error, the only test case I could think of is the case where array element is a constant. So my code checks if data object is a constant and if yes, it throws the error. Please correct me if it has to be implemented in a different way.

There's constraints (C924) on array-element as well - it needs to be a scalar (rank 0) data-ref. For a variable, see C901, but yes, it just means that it cannot be an element of a named constant. A variable can also be a reference to a function returning a data pointer, but I suspect that case is an error in the standard -- it couldn't be called at compilation time.

  1. C879 (R841) The scalar-structure-component shall be a variable. According to my understanding, the code for C878 should apply for C879 as well (as it checks whether the DataRef is a constant or not). I tried writing a testcase where a scalar component of a struture is a constant with the snippet below: type newNum integer a end type type(newNum), parameter :: num In this case, I see that num%a is not identified as a constant expression by F18. Shouldn't it be identified? If so, it requires modification in the implementation of IsConstantExpr

Every component in the derived type has to be initialized in the either the definition of the derived type or in the structure constructor used to initialized the named constant. This code snippet is in error (C807); if we check C807 already, there's a bug. IsConstantExpr is probably failing because there's no constant instance of the derived type to work with here.

Please correct me if I am interpreting things wrong. Thank you ,

anchuraj commented 4 years ago

Hi, @klausler , @tskeith, I would like to get clarifications on two constraints

  1. C878 (R841) The array-element shall be a variable. To throw the error, the only test case I could think of is the case where array element is a constant. So my code checks if data object is a constant and if yes, it throws the error. Please correct me if it has to be implemented in a different way.

There's constraints (C924) on array-element as well - it needs to be a scalar (rank 0) data-ref. For a variable, see C901, but yes, it just means that it cannot be an element of a named constant. A variable can also be a reference to a function returning a data pointer, but I suspect that case is an error in the standard -- it couldn't be called at compilation time.

  1. C879 (R841) The scalar-structure-component shall be a variable. According to my understanding, the code for C878 should apply for C879 as well (as it checks whether the DataRef is a constant or not). I tried writing a testcase where a scalar component of a struture is a constant with the snippet below: type newNum integer a end type type(newNum), parameter :: num In this case, I see that num%a is not identified as a constant expression by F18. Shouldn't it be identified? If so, it requires modification in the implementation of IsConstantExpr

Every component in the derived type has to be initialized in the either the definition of the derived type or in the structure constructor used to initialized the named constant. This code snippet is in error (C807); if we check C807 already, there's a bug. IsConstantExpr is probably failing because there's no constant instance of the derived type to work with here.

Please correct me if I am interpreting things wrong. Thank you ,

Hi, Thank you for the response. Sorry, I forgot to initialize it when I simplified the example for mentioning here. The example I used for testing C879 is the following:

type specialNumbers
  integer num
end type
type(specialNumbers), parameter ::newNumsArray(2) = (/ SpecialNumbers(1),  SpecialNumbers(2) /)
DATA (newNumsArray(i)%num, i = 1, 2) / 2 * specialNumbers(1) /

Here, I cheked whether newNumsArray(i)%num is a constant using IsConstantExpr and it was returning false.

klausler commented 4 years ago

Here, I cheked whether newNumsArray(i)%num is a constant using IsConstantExpr and it was returning false.

That's probably because IsConstantExpr doesn't think that implied DO variables are constants; it probably works with a component reference to a scalar named constant, and it's a bug if it doesn't.

anchuraj commented 4 years ago

Here, I cheked whether newNumsArray(i)%num is a constant using IsConstantExpr and it was returning false.

That's probably because IsConstantExpr doesn't think that implied DO variables are constants; it probably works with a component reference to a scalar named constant, and it's a bug if it doesn't. Implied do variables are treated as constants by IsConstantExpr. Consider the following code snippet (which is semantically wrong again)

type(specialNumbers), parameter ::newNum = SpecialNumbers(1)
DATA (newNum%num, i = 1, 2) / 2 * 1/

Here IsConstantExpr on newNum%num also returns false.

klausler commented 4 years ago

Here, I cheked whether newNumsArray(i)%num is a constant using IsConstantExpr and it was returning false.

That's probably because IsConstantExpr doesn't think that implied DO variables are constants; it probably works with a component reference to a scalar named constant, and it's a bug if it doesn't. Implied do variables are treated as constants by IsConstantExpr. Consider the following code snippet (which is semantically wrong again)

type(specialNumbers), parameter ::newNum = SpecialNumbers(1)
DATA (newNum%num, i = 1, 2) / 2 * 1/

Here IsConstantExpr on newNum%num also returns false.

Ok, thanks for checking. That's a bug. Want me to fix it for you, or would you like to do it?

anchuraj commented 4 years ago

Here, I cheked whether newNumsArray(i)%num is a constant using IsConstantExpr and it was returning false.

That's probably because IsConstantExpr doesn't think that implied DO variables are constants; it probably works with a component reference to a scalar named constant, and it's a bug if it doesn't. Implied do variables are treated as constants by IsConstantExpr. Consider the following code snippet (which is semantically wrong again)

type(specialNumbers), parameter ::newNum = SpecialNumbers(1)
DATA (newNum%num, i = 1, 2) / 2 * 1/

Here IsConstantExpr on newNum%num also returns false.

Ok, thanks for checking. That's a bug. Want me to fix it for you, or would you like to do it?

Thank you for the clarification. I will try to fix it and will get back if I have any doubts.

anchuraj commented 4 years ago

Thank you @tskeith, @kiranchandramohan for the comments. I have addressed your concerns. But build is failing only on arm64 gcc on a file which I have not edited.

[36/190] Building CXX object lib/Parser/CMakeFiles/FortranParser.dir/debug-parser.cpp.o | 1733s
-- | --
5217 | [37/190] Building CXX object lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o | 1767s
5218 | FAILED: lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o | 1768s
5219 | /usr/local/bin/g++   -I../include -Iinclude -isystem ../llvm-project/install/include -UNDEBUG -std=c++17 -fno-rtti -fno-exceptions -pedantic -Wall -Wextra -Werror -Wcast-qual -Wimplicit-fallthrough -Wdelete-non-virtual-dtor -O2   -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -MD -MT lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o -MF lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o.d -o lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o -c ../lib/Evaluate/fold-real.cpp | 1768s
5220 | g++: fatal error: Killed signal terminated program cc1plus | 1768s
5221 | compilation terminated. | 1768s
5222 | [38/190] Building CXX object lib/Evaluate/CMakeFiles/FortranEvaluate.dir/variable.cpp.o
anchuraj commented 4 years ago

Thank you @tskeith, @kiranchandramohan for the comments. I have addressed your concerns. But build is failing only on arm64 gcc on a file which I have not edited.

[36/190] Building CXX object lib/Parser/CMakeFiles/FortranParser.dir/debug-parser.cpp.o | 1733s
-- | --
5217 | [37/190] Building CXX object lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o | 1767s
5218 | FAILED: lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o | 1768s
5219 | /usr/local/bin/g++   -I../include -Iinclude -isystem ../llvm-project/install/include -UNDEBUG -std=c++17 -fno-rtti -fno-exceptions -pedantic -Wall -Wextra -Werror -Wcast-qual -Wimplicit-fallthrough -Wdelete-non-virtual-dtor -O2   -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -MD -MT lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o -MF lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o.d -o lib/Evaluate/CMakeFiles/FortranEvaluate.dir/fold-real.cpp.o -c ../lib/Evaluate/fold-real.cpp | 1768s
5220 | g++: fatal error: Killed signal terminated program cc1plus | 1768s
5221 | compilation terminated. | 1768s
5222 | [38/190] Building CXX object lib/Evaluate/CMakeFiles/FortranEvaluate.dir/variable.cpp.o

I pulled in recent changes and the build passed. I am still not sure why it failed.

anchuraj commented 4 years ago

Closing the pull request for now and will reopen in phabricator.