How to implement a Sematic Check in Flang

.. contents::
   :local:

I recently added a semantic check to the f18 compiler front end. This document describes my thought process and the resulting implementation.

For more information about the compiler, start with the compiler overview.

Problem definition

In the 2018 Fortran standard, section 11.1.7.4.3, paragraph 2, states that:

Except for the incrementation of the DO variable that occurs in step (3), the DO variable 
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(OUT) or INTENT(INOUT). I implemented this semantic check. Specifically, I changed the compiler to emit an error message if an active DO variable was passed to a dummy argument of a FUNCTION with INTENT(OUT). Similarly, I had the compiler emit a warning if an active DO variable was passed to a dummy argument with INTENT(INOUT). Previously, I had implemented similar checks for SUBROUTINE calls.

Creating a test

My first step was to create a test case to cause the problem. I called it testfun.f90 and used it to check the behavior of other Fortran compilers. Here's the initial version:

  subroutine s()
    Integer :: ivar, jvar

    do ivar = 1, 10
      jvar = intentOutFunc(ivar) ! Error since ivar is a DO variable
    end do

  contains
    function intentOutFunc(dummyArg)
      integer, intent(out) :: dummyArg
      integer  :: intentOutFunc

      dummyArg = 216
    end function intentOutFunc
  end subroutine s

I verified that other Fortran compilers produced an error message at the point of the call to intentOutFunc():

      jvar = intentOutFunc(ivar) ! Error since ivar is a DO variable

I also used this program to produce a parse tree for the program using the command:

  f18 -fdebug-dump-parse-tree -fparse-only testfun.f90

Here's the relevant fragment of the parse tree produced by the compiler:

| | ExecutionPartConstruct -> ExecutableConstruct -> DoConstruct
| | | NonLabelDoStmt
| | | | LoopControl -> LoopBounds
| | | | | Scalar -> Name = 'ivar'
| | | | | Scalar -> Expr = '1_4'
| | | | | | LiteralConstant -> IntLiteralConstant = '1'
| | | | | Scalar -> Expr = '10_4'
| | | | | | LiteralConstant -> IntLiteralConstant = '10'
| | | Block
| | | | ExecutionPartConstruct -> ExecutableConstruct -> ActionStmt -> AssignmentStmt = 'jvar=intentoutfunc(ivar)'
| | | | | Variable -> Designator -> DataRef -> Name = 'jvar'
| | | | | Expr = 'intentoutfunc(ivar)'
| | | | | | FunctionReference -> Call
| | | | | | | ProcedureDesignator -> Name = 'intentoutfunc'
| | | | | | | ActualArgSpec
| | | | | | | | ActualArg -> Expr = 'ivar'
| | | | | | | | | Designator -> DataRef -> Name = 'ivar'
| | | EndDoStmt -> 

Note that this fragment of the tree only shows four parser::Expr nodes, but the full parse tree also contained a fifth parser::Expr node for the constant 216 in the statement:

      dummyArg = 216

Analysis and implementation planning

I then considered what I needed to do. I needed to detect situations where an active DO variable was passed to a dummy argument with INTENT(OUT) or INTENT(INOUT). Once I detected such a situation, I needed to produce a message that highlighted the erroneous source code.

Deciding where to add the code to the compiler

This new semantic check would depend on several types of information -- the parse tree, source code location information, symbols, and expressions. Thus I needed to put my new code in a place in the compiler after the parse tree had been created, name resolution had already happened, and expression semantic checking had already taken place.

Most semantic checks for statements are implemented by walking the parse tree and performing analysis on the nodes they visit. My plan was to use this method. The infrastructure for walking the parse tree for statement semantic checking is implemented in the files lib/Semantics/semantics.cpp. Here's a fragment of the declaration of the framework's parse tree visitor from lib/Semantics/semantics.cpp:

  // A parse tree visitor that calls Enter/Leave functions from each checker
  // class C supplied as template parameters. Enter is called before the node's
  // children are visited, Leave is called after. No two checkers may have the
  // same Enter or Leave function. Each checker must be constructible from
  // SemanticsContext and have BaseChecker as a virtual base class.
  template<typename... C> class SemanticsVisitor : public virtual C... {
  public:
    using C::Enter...;
    using C::Leave...;
    using BaseChecker::Enter;
    using BaseChecker::Leave;
    SemanticsVisitor(SemanticsContext &context)
      : C{context}..., context_{context} {}
      ...

Since FUNCTION calls are a kind of expression, I was planning to base my implementation on the contents of parser::Expr nodes. I would need to define either an Enter() or Leave() function whose parameter was a parser::Expr node. Here's the declaration I put into lib/Semantics/check-do.h:

  void Leave(const parser::Expr &);

The Enter() functions get called at the time the node is first visited -- that is, before its children. The Leave() function gets called after the children are visited. For my check the visitation order didn't matter, so I arbitrarily chose to implement the Leave() function to visit the parse tree node.

Since my semantic check was focused on DO CONCURRENT statements, I added it to the file lib/Semantics/check-do.cpp where most of the semantic checking for DO statements already lived.

Taking advantage of prior work

When implementing a similar check for SUBROUTINE calls, I created a utility functions in lib/Semantics/semantics.cpp to emit messages if a symbol corresponding to an active DO variable was being potentially modified:

  void WarnDoVarRedefine(const parser::CharBlock &location, const Symbol &var);
  void CheckDoVarRedefine(const parser::CharBlock &location, const Symbol &var);

The first function is intended for dummy arguments of INTENT(INOUT) and the second for INTENT(OUT).

Thus I needed three pieces of information --

1. the source location of the erroneous text,
2. the INTENT of the associated dummy argument, and
3. the relevant symbol passed as the actual argument.

The first and third are needed since they're required to call the utility functions. The second is needed to determine whether to call them.

Finding the source location

The source code location information that I'd need for the error message must come from the parse tree. I looked in the file include/flang/Parser/parse-tree.h and determined that a struct Expr contained source location information since it had the field CharBlock source. Thus, if I visited a parser::Expr node, I could get the source location information for the associated expression.

Determining the INTENT

I knew that I could find the INTENT of the dummy argument associated with the actual argument from the function called dummyIntent() in the class evaluate::ActualArgument in the file include/flang/Evaluate/call.h. So if I could find an evaluate::ActualArgument in an expression, I could determine the INTENT of the associated dummy argument. I knew that it was valid to call dummyIntent() because the data on which dummyIntent() depends is established during semantic processing for expressions, and the semantic processing for expressions happens before semantic checking for DO constructs.

In my prior work on checking the INTENT of arguments for SUBROUTINE calls, the parse tree held a node for the call (a parser::CallStmt) that contained an evaluate::ProcedureRef node.

  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. I could then find the INTENT of a dummy argument from the evaluate::ActualArgument node.

For a FUNCTION call, though, there is no similar way to get from a parse tree node to an evaluate::ProcedureRef node. But I knew that there was an existing framework used in DO construct semantic checking that traversed an evaluate::Expr node collecting semantics::Symbol nodes. I guessed that I'd be able to use a similar framework to traverse an evaluate::Expr node to find all of the evaluate::ActualArgument nodes.

Note that the compiler has multiple types called Expr. One is in the parser namespace. parser::Expr is defined in the file include/flang/Parser/parse-tree.h. It represents a parsed expression that maps directly to the source code and has fields that specify any operators in the expression, the operands, and the source position of the expression.

Additionally, in the namespace evaluate, there are evaluate::Expr<T> template classes defined in the file include/flang/Evaluate/expression.h. These are parameterized over the various types of Fortran and constitute a suite of strongly-typed representations of valid Fortran expressions of type T that have been fully elaborated with conversion operations and subjected to constant folding. After an expression has undergone semantic analysis, the field typedExpr in the parser::Expr node is filled in with a pointer that owns an instance of evaluate::Expr<SomeType>, the most general representation of an analyzed expression.

All of the declarations associated with both FUNCTION and SUBROUTINE calls are in include/flang/Evaluate/call.h. An evaluate::FunctionRef inherits from an evaluate::ProcedureRef which contains the list of evaluate::ActualArgument nodes. But the relationship between an evaluate::FunctionRef node and its associated arguments is not relevant. I only needed to find the evaluate::ActualArgument nodes in an expression. They hold all of the information I needed.

So my plan was to start with the parser::Expr node and extract its associated evaluate::Expr field. I would then traverse the evaluate::Expr tree collecting all of the evaluate::ActualArgument nodes. I would look at each of these nodes to determine the INTENT of the associated dummy argument.

This combination of the traversal framework and dummyIntent() would give me the INTENT of all of the dummy arguments in a FUNCTION call. Thus, I would have the second piece of information I needed.

Determining if the actual argument is a variable

I also guessed that I could determine if the evaluate::ActualArgument consisted of a variable.

Once I had a symbol for the variable, I could call one of the functions:

  void WarnDoVarRedefine(const parser::CharBlock &, const Symbol &);
  void CheckDoVarRedefine(const parser::CharBlock &, const Symbol &);

to emit the messages.

If my plans worked out, this would give me the three pieces of information I needed -- the source location of the erroneous text, the INTENT of the dummy argument, and a symbol that I could use to determine whether the actual argument was an active DO variable.

Implementation

Adding a parse tree visitor

I started my implementation by adding a visitor for parser::Expr nodes. Since this analysis is part of DO construct checking, I did this in lib/Semantics/check-do.cpp. I added a print statement to the visitor to verify that my new code was actually getting executed.

In lib/Semantics/check-do.h, I added the declaration for the visitor:

  void Leave(const parser::Expr &);

In lib/Semantics/check-do.cpp, I added an (almost empty) implementation:

  void DoChecker::Leave(const parser::Expr &) {
    std::cout << "In Leave for parser::Expr\n";
  }

I then built the compiler with these changes and ran it on my test program. This time, I made sure to invoke semantic checking. Here's the command I used:

  f18 -fdebug-resolve-names -fdebug-dump-parse-tree -funparse-with-symbols testfun.f90

This produced the output:

  In Leave for parser::Expr
  In Leave for parser::Expr
  In Leave for parser::Expr
  In Leave for parser::Expr
  In Leave for parser::Expr

This made sense since the parse tree contained five parser::Expr nodes. So far, so good. Note that a parse::Expr node has a field with the source position of the associated expression (CharBlock source). So I now had one of the three pieces of information needed to detect and report errors.

Collecting the actual arguments

To get the INTENT of the dummy arguments and the semantics::Symbol associated with the actual argument, I needed to find all of the actual arguments embedded in an expression that contained a FUNCTION call. So my next step was to write the framework to walk the evaluate::Expr to gather all of the evaluate::ActualArgument nodes. The code that I planned to model it on was the existing infrastructure that collected all of the semantics::Symbol nodes from an evaluate::Expr. I found this implementation in lib/Evaluate/tools.cpp:

  struct CollectSymbolsHelper
    : public SetTraverse<CollectSymbolsHelper, semantics::SymbolSet> {
    using Base = SetTraverse<CollectSymbolsHelper, semantics::SymbolSet>;
    CollectSymbolsHelper() : Base{*this} {}
    using Base::operator();
    semantics::SymbolSet operator()(const Symbol &symbol) const {
      return {symbol};
    }
  };
  template<typename A> semantics::SymbolSet CollectSymbols(const A &x) {
    return CollectSymbolsHelper{}(x);
  }

Note that the CollectSymbols() function returns a semantics::Symbolset, which is declared in include/flang/Semantics/symbol.h:

  using SymbolSet = std::set<SymbolRef>;

This infrastructure yields a collection based on std::set<>. Using an std::set<> means that if the same object is inserted twice, the collection only gets one copy. This was the behavior that I wanted.

Here's a sample invocation of CollectSymbols() that I found:

    if (const auto *expr{GetExpr(parsedExpr)}) {
      for (const Symbol &symbol : evaluate::CollectSymbols(*expr)) {

I noted that a SymbolSet did not actually contain an std::set<Symbol>. This wasn't surprising since we don't want to put the full semantics::Symbol objects into the set. Ideally, we would be able to create an std::set<Symbol &> (a set of C++ references to symbols). But C++ doesn't support sets that contain references. This limitation is part of the rationale for the f18 implementation of type common::Reference, which is defined in include/flang/Common/reference.h.

SymbolRef, the specialization of the template common::Reference for semantics::Symbol, is declared in the file include/flang/Semantics/symbol.h:

  using SymbolRef = common::Reference<const Symbol>;

So to implement something that would collect evaluate::ActualArgument nodes from an evaluate::Expr, I first defined the required types ActualArgumentRef and ActualArgumentSet. Since these are being used exclusively for DO construct semantic checking (currently), I put their definitions into lib/Semantics/check-do.cpp:

  namespace Fortran::evaluate {
    using ActualArgumentRef = common::Reference<const ActualArgument>;
  }


  using ActualArgumentSet = std::set<evaluate::ActualArgumentRef>;

Since ActualArgument is in the namespace evaluate, I put the definition for ActualArgumentRef in that namespace, too.

I then modeled the code to create an ActualArgumentSet after the code to collect a SymbolSet and put it into lib/Semantics/check-do.cpp:

  struct CollectActualArgumentsHelper
    : public evaluate::SetTraverse<CollectActualArgumentsHelper,
          ActualArgumentSet> {
    using Base = SetTraverse<CollectActualArgumentsHelper, ActualArgumentSet>;
    CollectActualArgumentsHelper() : Base{*this} {}
    using Base::operator();
    ActualArgumentSet operator()(const evaluate::ActualArgument &arg) const {
      return ActualArgumentSet{arg};
    }
  };

  template<typename A> ActualArgumentSet CollectActualArguments(const A &x) {
    return CollectActualArgumentsHelper{}(x);
  }

  template ActualArgumentSet CollectActualArguments(const SomeExpr &);

Unfortunately, when I tried to build this code, I got an error message saying std::set requires the < operator to be defined for its contents. To fix this, I added a definition for <. I didn't care how < was defined, so I just used the address of the object:

  inline bool operator<(ActualArgumentRef x, ActualArgumentRef y) {
    return &*x < &*y;
  }

I was surprised when this did not make the error message saying that I needed the < operator go away. Eventually, I figured out that the definition of the < operator needed to be in the evaluate namespace. Once I put it there, everything compiled successfully. Here's the code that worked:

  namespace Fortran::evaluate {
  using ActualArgumentRef = common::Reference<const ActualArgument>;

  inline bool operator<(ActualArgumentRef x, ActualArgumentRef y) {
    return &*x < &*y;
  }
  }

I then modified my visitor for the parser::Expr to invoke my new collection framework. To verify that it was actually doing something, I printed out the number of evaluate::ActualArgument nodes that it collected. Note the call to GetExpr() in the invocation of CollectActualArguments(). I modeled this on similar code that collected a SymbolSet described above:

  void DoChecker::Leave(const parser::Expr &parsedExpr) {
    std::cout << "In Leave for parser::Expr\n";
    ActualArgumentSet argSet{CollectActualArguments(GetExpr(parsedExpr))};
    std::cout << "Number of arguments: " << argSet.size() << "\n";
  }

I compiled and tested this code on my little test program. Here's the output that I got:

  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 1
  In Leave for parser::Expr
  Number of arguments: 0

So most of the parser::Exprnodes contained no actual arguments, but the fourth expression in the parse tree walk contained a single argument. This may seem wrong since the third parser::Expr node in the file contains the FunctionReference node along with the arguments that we're gathering. But since the tree walk function is being called upon leaving a parser::Expr node, the function visits the parser::Expr node associated with the parser::ActualArg node before it visits the parser::Expr node associated with the parser::FunctionReference node.

So far, so good.

Finding the INTENT of the dummy argument

I now wanted to find the INTENT of the dummy argument associated with the arguments in the set. As mentioned earlier, the type evaluate::ActualArgument has a member function called dummyIntent() that gives this value. So I augmented my code to print out the INTENT:

  void DoChecker::Leave(const parser::Expr &parsedExpr) {
    std::cout << "In Leave for parser::Expr\n";
    ActualArgumentSet argSet{CollectActualArguments(GetExpr(parsedExpr))};
    std::cout << "Number of arguments: " << argSet.size() << "\n";
    for (const evaluate::ActualArgumentRef &argRef : argSet) {
      common::Intent intent{argRef->dummyIntent()};
      switch (intent) {
        case common::Intent::In: std::cout << "INTENT(IN)\n"; break;
        case common::Intent::Out: std::cout << "INTENT(OUT)\n"; break;
        case common::Intent::InOut: std::cout << "INTENT(INOUT)\n"; break;
        default: std::cout << "default INTENT\n";
      }
    }
  }

I then rebuilt my compiler and ran it on my test case. This produced the following output:

  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 1
  INTENT(OUT)
  In Leave for parser::Expr
  Number of arguments: 0

I then modified my test case to convince myself that I was getting the correct INTENT for IN, INOUT, and default cases.

So far, so good.

Finding the symbols for arguments that are variables

The third and last piece of information I needed was to determine if a variable was being passed as an actual argument. In such cases, I wanted to get the symbol table node (semantics::Symbol) for the variable. My starting point was the evaluate::ActualArgument node.

I was unsure of how to do this, so I browsed through existing code to look for how it treated evaluate::ActualArgument objects. Since most of the code that deals with the evaluate namespace is in the lib/Evaluate directory, I looked there. I ran grep on all of the .cpp files looking for uses of ActualArgument. One of the first hits I got was in lib/Evaluate/call.cpp in the definition of ActualArgument::GetType():

std::optional<DynamicType> ActualArgument::GetType() const {
  if (const Expr<SomeType> *expr{UnwrapExpr()}) {
    return expr->GetType();
  } else if (std::holds_alternative<AssumedType>(u_)) {
    return DynamicType::AssumedType();
  } else {
    return std::nullopt;
  }
}

I noted the call to UnwrapExpr() that yielded a value of Expr<SomeType>. So I guessed that I could use this member function to get an evaluate::Expr<SomeType> on which I could perform further analysis.

I also knew that the header file include/flang/Evaluate/tools.h held many utility functions for dealing with evaluate::Expr objects. I was hoping to find something that would determine if an evaluate::Expr was a variable. So I searched for IsVariable and got a hit immediately.

  template<typename A> bool IsVariable(const A &x) {
    if (auto known{IsVariableHelper{}(x)}) {
      return *known;
    } else {
      return false;
    }
  }

But I actually needed more than just the knowledge that an evaluate::Expr was a variable. I needed the semantics::Symbol associated with the variable. So I searched in include/flang/Evaluate/tools.h for functions that returned a semantics::Symbol. I found the following:

// If an expression is simply a whole symbol data designator,
// extract and return that symbol, else null.
template<typename A> const Symbol *UnwrapWholeSymbolDataRef(const A &x) {
  if (auto dataRef{ExtractDataRef(x)}) {
    if (const SymbolRef * p{std::get_if<SymbolRef>(&dataRef->u)}) {
      return &p->get();
    }
  }
  return nullptr;
}

This was exactly what I wanted. DO variables must be whole symbols. So I could try to extract a whole semantics::Symbol from the evaluate::Expr in my evaluate::ActualArgument. If this extraction resulted in a semantics::Symbol that wasn't a nullptr, I could then conclude if it was a variable that I could pass to existing functions that would determine if it was an active DO variable.

I then modified the compiler to perform the analysis that I'd guessed would work:

  void DoChecker::Leave(const parser::Expr &parsedExpr) {
    std::cout << "In Leave for parser::Expr\n";
    ActualArgumentSet argSet{CollectActualArguments(GetExpr(parsedExpr))};
    std::cout << "Number of arguments: " << argSet.size() << "\n";
    for (const evaluate::ActualArgumentRef &argRef : argSet) {
      if (const SomeExpr * argExpr{argRef->UnwrapExpr()}) {
        std::cout << "Got an unwrapped Expr\n";
        if (const Symbol * var{evaluate::UnwrapWholeSymbolDataRef(*argExpr)}) {
          std::cout << "Found a whole variable: " << *var << "\n";
        }
      }
      common::Intent intent{argRef->dummyIntent()};
      switch (intent) {
        case common::Intent::In: std::cout << "INTENT(IN)\n"; break;
        case common::Intent::Out: std::cout << "INTENT(OUT)\n"; break;
        case common::Intent::InOut: std::cout << "INTENT(INOUT)\n"; break;
        default: std::cout << "default INTENT\n";
      }
    }
  }

Note the line that prints out the symbol table entry for the variable:

          std::cout << "Found a whole variable: " << *var << "\n";

The compiler defines the "<<" operator for semantics::Symbol, which is handy for analyzing the compiler's behavior.

Here's the result of running the modified compiler on my Fortran test case:

  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 1
  Got an unwrapped Expr
  Found a whole variable: ivar: ObjectEntity type: INTEGER(4)
  INTENT(OUT)
  In Leave for parser::Expr
  Number of arguments: 0

Sweet.

Emitting the messages

At this point, using the source location information from the original parser::Expr, I had enough information to plug into the exiting interfaces for emitting messages for active DO variables. I modified the compiler code accordingly:

  void DoChecker::Leave(const parser::Expr &parsedExpr) {
    std::cout << "In Leave for parser::Expr\n";
    ActualArgumentSet argSet{CollectActualArguments(GetExpr(parsedExpr))};
    std::cout << "Number of arguments: " << argSet.size() << "\n";
    for (const evaluate::ActualArgumentRef &argRef : argSet) {
      if (const SomeExpr * argExpr{argRef->UnwrapExpr()}) {
        std::cout << "Got an unwrapped Expr\n";
        if (const Symbol * var{evaluate::UnwrapWholeSymbolDataRef(*argExpr)}) {
          std::cout << "Found a whole variable: " << *var << "\n";
          common::Intent intent{argRef->dummyIntent()};
          switch (intent) {
            case common::Intent::In: std::cout << "INTENT(IN)\n"; break;
            case common::Intent::Out: 
              std::cout << "INTENT(OUT)\n"; 
              context_.CheckDoVarRedefine(parsedExpr.source, *var);
              break;
            case common::Intent::InOut: 
              std::cout << "INTENT(INOUT)\n"; 
              context_.WarnDoVarRedefine(parsedExpr.source, *var);
              break;
            default: std::cout << "default INTENT\n";
          }
        }
      }
    }
  }

I then ran this code on my test case, and miraculously, got the following output:

  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 0
  In Leave for parser::Expr
  Number of arguments: 1
  Got an unwrapped Expr
  Found a whole variable: ivar: ObjectEntity type: INTEGER(4)
  INTENT(OUT)
  In Leave for parser::Expr
  Number of arguments: 0
  testfun.f90:6:12: error: Cannot redefine DO variable 'ivar'
        jvar = intentOutFunc(ivar)
               ^^^^^^^^^^^^^^^^^^^
  testfun.f90:5:6: Enclosing DO construct
      do ivar = 1, 10
         ^^^^

Even sweeter.

Improving the test case

At this point, my implementation seemed to be working. But I was concerned about the limitations of my test case. So I augmented it to include arguments other than INTENT(OUT) and more complex expressions. Luckily, my augmented test did not reveal any new problems.

Here's the test I ended up with:

  subroutine s()

    Integer :: ivar, jvar

    ! This one is OK
    do ivar = 1, 10
      jvar = intentInFunc(ivar)
    end do

    ! Error for passing a DO variable to an INTENT(OUT) dummy
    do ivar = 1, 10
      jvar = intentOutFunc(ivar)
    end do

    ! Error for passing a DO variable to an INTENT(OUT) dummy, more complex 
    ! expression
    do ivar = 1, 10
      jvar = 83 + intentInFunc(intentOutFunc(ivar))
    end do

    ! Warning for passing a DO variable to an INTENT(INOUT) dummy
    do ivar = 1, 10
      jvar = intentInOutFunc(ivar)
    end do

  contains
    function intentInFunc(dummyArg)
      integer, intent(in) :: dummyArg
      integer  :: intentInFunc

      intentInFunc = 343
    end function intentInFunc

    function intentOutFunc(dummyArg)
      integer, intent(out) :: dummyArg
      integer  :: intentOutFunc

      dummyArg = 216
      intentOutFunc = 343
    end function intentOutFunc

    function intentInOutFunc(dummyArg)
      integer, intent(inout) :: dummyArg
      integer  :: intentInOutFunc

      dummyArg = 216
      intentInOutFunc = 343
    end function intentInOutFunc

  end subroutine s

Submitting the pull request

At this point, my implementation seemed functionally complete, so I stripped out all of the debug statements, ran clang-format on it and reviewed it to make sure that the names were clear. Here's what I ended up with:

  void DoChecker::Leave(const parser::Expr &parsedExpr) {
    ActualArgumentSet argSet{CollectActualArguments(GetExpr(parsedExpr))};
    for (const evaluate::ActualArgumentRef &argRef : argSet) {
      if (const SomeExpr * argExpr{argRef->UnwrapExpr()}) {
        if (const Symbol * var{evaluate::UnwrapWholeSymbolDataRef(*argExpr)}) {
          common::Intent intent{argRef->dummyIntent()};
          switch (intent) {
            case common::Intent::Out: 
              context_.CheckDoVarRedefine(parsedExpr.source, *var);
              break;
            case common::Intent::InOut: 
              context_.WarnDoVarRedefine(parsedExpr.source, *var);
              break;
            default:; // INTENT(IN) or default intent
          }
        }
      }
    }
  }

I then created a pull request to get review comments.

Responding to pull request comments

I got feedback suggesting that I use an if statement rather than a case statement. Another comment reminded me that I should look at the code I'd previously writted to do a similar check for SUBROUTINE calls to see if there was an opportunity to share code. This examination resulted in converting my existing code to the following pair of functions:

  static void CheckIfArgIsDoVar(const evaluate::ActualArgument &arg,
      const parser::CharBlock location, SemanticsContext &context) {
    common::Intent intent{arg.dummyIntent()};
    if (intent == common::Intent::Out || intent == common::Intent::InOut) {
      if (const SomeExpr * argExpr{arg.UnwrapExpr()}) {
        if (const Symbol * var{evaluate::UnwrapWholeSymbolDataRef(*argExpr)}) {
          if (intent == common::Intent::Out) {
            context.CheckDoVarRedefine(location, *var);
          } else {
            context.WarnDoVarRedefine(location, *var);  // INTENT(INOUT)
          }
        }
      }
    }
  }

  void DoChecker::Leave(const parser::Expr &parsedExpr) {
    if (const SomeExpr * expr{GetExpr(parsedExpr)}) {
      ActualArgumentSet argSet{CollectActualArguments(*expr)};
      for (const evaluate::ActualArgumentRef &argRef : argSet) {
        CheckIfArgIsDoVar(*argRef, parsedExpr.source, context_);
      }
    }
  }

The function CheckIfArgIsDoVar() was shared with the checks for DO variables being passed to SUBROUTINE calls.

At this point, my pull request was approved, and I merged it and deleted the associated branch.