feat: add parsing support for INTERFACE-ID, METHOD-ID, class body and INVOKE statement#293
feat: add parsing support for INTERFACE-ID, METHOD-ID, class body and INVOKE statement#293kmr-srbh wants to merge 19 commits into
INTERFACE-ID, METHOD-ID, class body and INVOKE statement#293Conversation
df0bfcc to
dd8db31
Compare
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
4fdc883 to
750de7e
Compare
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
7ee4e80 to
47cbb60
Compare
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
| case COB_MODULE_TYPE_PROGRAM: return "program"; | ||
| case COB_MODULE_TYPE_FUNCTION: return "function"; | ||
| case COB_MODULE_TYPE_CLASS: return "class"; | ||
| case COB_MODULE_TYPE_INTERFACE: return "interface"; | ||
| case COB_MODULE_TYPE_METHOD: return "method"; |
There was a problem hiding this comment.
Note these may deserve translations. Surrounding the string literals with _( and ) should do the job.
There was a problem hiding this comment.
The main question is: do we need `to output the type at all or isn't it enough to have a note about the previous definition?
Note: there is a "general" function that outputs the "type" of a tree, we possibly want to adjust that in any case checking the tree's type there; I guess most of the strings there are not msgid's but string constants as those are taken from the COBOL standard, which is possibly what we want here as well.
There was a problem hiding this comment.
Yes this is a debatable point. The use of these nouns in the current code is in error messages that were fully translated before, where the type of compilation unit appeared clearly. The growing amount of elements for this type may however incur a lot of code duplication where such messages are emitted…
| case COB_MODULE_TYPE_PROGRAM: return "program"; | ||
| case COB_MODULE_TYPE_FUNCTION: return "function"; | ||
| case COB_MODULE_TYPE_CLASS: return "class"; | ||
| case COB_MODULE_TYPE_INTERFACE: return "interface"; | ||
| case COB_MODULE_TYPE_METHOD: return "method"; |
There was a problem hiding this comment.
The main question is: do we need `to output the type at all or isn't it enough to have a note about the previous definition?
Note: there is a "general" function that outputs the "type" of a tree, we possibly want to adjust that in any case checking the tree's type there; I guess most of the strings there are not msgid's but string constants as those are taken from the COBOL standard, which is possibly what we want here as well.
|
Hi guys, just checking for the timeline. The original proposal had the first coding period ending today with "Handle complete syntax for each construct as defined by the ISO standard." Not sure how far you are with "complete syntax" or may even need to reduce the amount of syntax parsed. The current GSoC timeline has the first coding period to July 6th - similar for the end of the second period being a week longer with the final end date August 24th. If both of you want to do longer, we can still extend the period as well, but only if we request this week, if I remember correctly. |
|
We have completed parser support for all the required paragraphs (with also some AST support and syntax checks) with this PR. For the parser, only Regarding the second phase of the coding period, I actually got it from here - https://developers.google.com/open-source/gsoc/timeline#july_6_-_august_16 where it says the work-period ends on August 16th. I might have made a mistake in understanding it. Edit: I think I would want to push the |
|
Update: I have addressed all the review suggestions locally and also implemented the |
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
|
Update: We fixed an important error in today's meeting. It turned out to be due to an external state variable affecting the scanner's output. The test from the ISO standard throws some errors related to variable names being undefined now, which is related to AST work and fine for now. I will push the test and update the changelog. |
b72b2c9 to
1b63c53
Compare
INTERFACE-ID, METHOD-ID, class body and INVOKE statement
|
Update: All tests pass locally. I can squash all commits after review. |
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
1b63c53 to
0659d5a
Compare
nberth
left a comment
There was a problem hiding this comment.
Thanks that's very good progress!
Just to validate: we want to keep the original plan (if not then @nberth may write to the GNU project GSoC org admins), doing the evaluation next week, right? (@nberth please write the evaluation and feel free to check via Matrix if you have any questions, otherwise drop a note and I'll have a second look afterwards) |
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
|
Update: Pushed changes with respect to the code review. Removed getter/setter AST node. Fixed tests for GET/SET syntax based on the MicroFocus docs (https://www.microfocus.com/documentation/reuze/60d/opmeth.htm#u040). Supported |
883248b to
dca64d3
Compare
|
Update: Got inline method invocation syntax working and added tests. I supported it as an expression with the already present |
Signed-off-by: Saurabh Kumar <developer.saurabh@outlook.com>
dca64d3 to
d698eea
Compare
| }, | ||
| { "CLASS", 0, 0, CLASS, /* 2002 */ | ||
| 0, 0 | ||
| CB_CS_CLASS_SPECIFIER, 0 |
There was a problem hiding this comment.
Now that I look at it a second time, I think there is an issue here as CLASS is a token that appears in several constructs of the grammar. So, entering the class-specifier special context in this way may not be the proper solution. It is quite possible a properly placed __CS_ENTER (CB_CS_CLASS_SPECIFIER); in the parser may be enough.
(This additionally says some more checks may be needed in the parser to detect every context is properly exited — or else that means CLASS (for "character"-class) is not used that often in the testsuite.)
| id_or_lit | ||
| call_using | ||
| call_returning | ||
| ; |
There was a problem hiding this comment.
Note: You may check the syntax rules for the invoke statement and try to implement at least those outside of the using/returning args as this will likely show a bunch of missing attributes in the used structures of the AST.
Checking the using/returning parameters for conformance is also useful, this part will show if the classes and methods are correctly stored in the internal "external repository".
Both checks should be done in typeck.c and be called from both invoke_statement and inline_method_invocation, which may both yield in nearly the same code generation later as well.
If possible I'd suggest to have both in before merging the OO changes upstream.
You guys will see together if that's reasonable and if more parts are necessary before that.
INTERFACE-ID,METHOD-ID,FACTORY,OBJECTparagraphs andINVOKEstatement. SupportCLASSand
INTERFACEspecifiers inREPOSITORYparagraph. Also supportkeywords
SELFandACTIVE-CLASS.consistency with names specified in the
REPOSITORYparagraph.