Explanation of how to implement a semantic check #939

psteinfeld · 2020-01-17T21:01:28Z

This is the story of implementing semantic checks for passing DO
variables to functions with dummy arguments with INTENT(OUT) or
INTENT(INOUT). I wrote it as an example of how to make changes
to the compiler for a newcomer.

kiranchandramohan

Thanks @psteinfeld for writing this document based on your experience of implementing the do index variable redefinition check. Should be helpful for new people writing semantic checks.

I like the title of this PR (Explanation of how to implement a semantic check) but the filename probably has to be modified to look something like the title of the PR.

What information would you have missed if you had the visitor at the ActualArgSpec level? Is it just the source location?

Have noted some minor nits inline.

kiranchandramohan · 2020-01-22T13:03:24Z

documentation/FrontEndTutorial.md

+shall neither be redefined nor become undefined while the DO construct is active.
+```
+One of the ways that DO variables might be redefined is if they are passed to
+functions with dummy arguments whose ```INTENT``` is ```INTENT(IN)``` or


INTENT(OUT)?

Good catch.

You can use single backquotes (grave accents) for inline code.

kiranchandramohan · 2020-01-22T13:08:29Z

documentation/FrontEndTutorial.md

+```INTENT``` of the dummy argument associated with the actual argument from the
+a function called ```dummyIntent()``` in the class


typo "the a function called"

kiranchandramohan · 2020-01-22T13:14:14Z

documentation/FrontEndTutorial.md

+nodes.  I would look at each of these nodes to determine the ```INTENT``` of
+the associated dummy argument.
+
+This combination of the traveral framework and ```dummyIntent()``` would give


typo traversal

psteinfeld · 2020-01-22T18:06:34Z

Thanks @psteinfeld for writing this document based on your experience of implementing the do index variable redefinition check. Should be helpful for new people writing semantic checks.

I like the title of this PR (Explanation of how to implement a semantic check) but the filename probably has to be modified to look something like the title of the PR.

What information would you have missed if you had the visitor at the ActualArgSpec level? Is it just the source location?

Have noted some minor nits inline.

Thanks for reviewing this, @kiranchandramohan.

I wasn't sure what to call the document. How about "ImplementingASemanticCheck.md"?

I probably should have discussed this in the document, but at one point I had a visitor that triggered on the type parser::ActualArg. As you mention, this node does not contain source information. But more importantly, the expression it contains does not contain any evaluate::ActualArgument nodes. These are needed to discover the intent of the dummy arguments. To get at these evaluate::ActualArgument nodes, I had to start with the expression that contained the function call.

kiranchandramohan · 2020-01-23T15:59:30Z

Thanks @psteinfeld for writing this document based on your experience of implementing the do index variable redefinition check. Should be helpful for new people writing semantic checks.
I like the title of this PR (Explanation of how to implement a semantic check) but the filename probably has to be modified to look something like the title of the PR.
What information would you have missed if you had the visitor at the ActualArgSpec level? Is it just the source location?
Have noted some minor nits inline.

Thanks for reviewing this, @kiranchandramohan.

I wasn't sure what to call the document. How about "ImplementingASemanticCheck.md"?

I probably should have discussed this in the document, but at one point I had a visitor that triggered on the type parser::ActualArg. As you mention, this node does not contain source information.

Since you have written in a conversational style it will be good to have this info that you went the parser::ActualArg path and found that the information that you need is not available. A word about why source information is there in some nodes and not there in others would also be useful.

Also, some information about how an expression models a function call might be useful. In the case of subroutines, you have a call statement so it is obvious there.

But more importantly, the expression it contains does not contain any evaluate::ActualArgument nodes. These are needed to discover the intent of the dummy arguments. To get at these evaluate::ActualArgument nodes, I had to start with the expression that contained the function call.

Are you hinting here that the Subroutine's arguments have the Actual Argument info but a Function's arguments do not have?
A simplistic first thought would be that once you get the argument list of the Function/Subroutine call the processing should be the same.

kiranchandramohan · 2020-01-23T16:15:29Z

A bit of time spent on where a semantic check should be added would also be good. That is probably the first thing that hits a first-time contributor. Some high-level guidance like if it is an Execution Construct it should be here or if it is a Declaration Construct it should be there. When should a person write his own checker and when not. Should a check be added during name or label resolution? Also in some cases, it is obvious like If it is something related to a do loop, OpenMP or co-array.

Feel free to ignore if this is outside the scope of this document.

psteinfeld · 2020-01-23T17:37:59Z

Since you have written in a conversational style it will be good to have this info that you went the parser::ActualArg path and found that the information that you need is not available. A word about why source information is there in some nodes and not there in others would also be useful.

Thanks, Kiran.

The very fact that you're wondering about this means that I should have included it in the document. I'll do that now. I plan to explain the difference between this case (for function calls), and the case for subroutine calls along with their relevant differences that affected the implementation. Please review again after I add this information.

Also, some information about how an expression models a function call might be useful. In the case of subroutines, you have a call statement so it is obvious there.

Will do.

But more importantly, the expression it contains does not contain any evaluate::ActualArgument nodes. These are needed to discover the intent of the dummy arguments. To get at these evaluate::ActualArgument nodes, I had to start with the expression that contained the function call.

Are you hinting here that the Subroutine's arguments have the Actual Argument info but a Function's arguments do not have?
A simplistic first thought would be that once you get the argument list of the Function/Subroutine call the processing should be the same.

I plan to explain this further in the addition I describe above, but here's a preview. In both cases, the most difficult piece of information I need is the INTENT of the dummy argument. This is accessible through the evaluate::ActualArgument. In the case of a subroutine call, the evaluate::ActualArgument nodes are accessible through the typedCall field of a parser::CallStmt:

struct CallStmt {
  WRAPPER_CLASS_BOILERPLATE(CallStmt, Call);
  mutable std::unique_ptr<evaluate::ProcedureRef,
      common::Deleter<evaluate::ProcedureRef>>
      typedCall;  // filled by semantics
};

The evaluate::ProcedureRef``` contains a list of evaluate::ActualArgument` nodes.

For a function call, though, there is no way to map from a parse tree node to an evaluate::ProcedureRef node. But I can take advantage of the fact that all function calls are embedded in expressions, either directly or indirectly. Once I have an expression, I can traverse it to gather all of the evaluate::ActualArgument nodes as described in the document.

For a parser::CallStmt node, there's no way (other than through the typedCall field) to get to the evaluate::ActualArgument node.

psteinfeld · 2020-01-23T17:40:33Z

A bit of time spent on where a semantic check should be added would also be good. That is probably the first thing that hits a first-time contributor. Some high-level guidance like if it is an Execution Construct it should be here or if it is a Declaration Construct it should be there. When should a person write his own checker and when not. Should a check be added during name or label resolution? Also in some cases, it is obvious like If it is something related to a do loop, OpenMP or co-array.

Feel free to ignore if this is outside the scope of this document.

I can certainly add a little more information about how I decided where to implement this check. I think it is out of the scope of this document to cover the topic generally.

kiranchandramohan

LGTM.

ichoyjx · 2020-01-31T07:30:27Z