Fix local name collisions between group cap dummy arguments and locals

scripts/metavar.py

@@ -1863,6 +1863,12 @@ def find_local_name(self, local_name, any_scope=False):
         # end if
         return pvar
 
+    def known_local_names(self):
+        """Return a list of the local names in use in this dictionary.
+        The returned names are lowercase because local name use is
+        case insensitive."""
+        return list(self.__local_names.keys())
+
     def find_error_variables(self, any_scope=False, clone_as_out=False):
         """Find and return a consistent set of error variables in this
         dictionary.
@@ -2098,19 +2104,27 @@ def var_call_string(self, var, loop_vars=None):
         """
         return var.call_string(self, loop_vars=loop_vars)
 
-    def new_internal_variable_name(self, prefix=None, max_len=63):
+    def new_internal_variable_name(self, prefix=None, max_len=63,
+                                   reserved=None):
         """Find a new local variable name for this dictionary.
         The new name begins with <prefix>_<self.name> or with <self.name>
         (where <self.name> is this VarDictionary's name) if <prefix> is None.
         The new variable name is kept to a maximum length of <max_len>.
+        If <reserved> is passed, it is a collection of local names (e.g.,
+        from a related dictionary) which the new name must also avoid.
         """
         index = 0
         if prefix is None:
             var_prefix = '{}_var'.format(self.name)
         else:
             var_prefix = '{}'.format(prefix)
         # end if
-        varlist = [x for x in self.__local_names.keys() if var_prefix in x]
+        # Local names are used case insensitively, compare in lowercase
+        varlist = [x for x in self.__local_names.keys()
+                   if var_prefix.lower() in x]
+        if reserved:
+            varlist.extend([x.lower() for x in reserved])
+        # end if
         newvar = None
         while newvar is None:
             if index == 0:
@@ -2122,7 +2136,7 @@ def new_internal_variable_name(self, prefix=None, max_len=63):
             if len(newvar) > max_len:
                 var_prefix = var_prefix[:-1]
                 newvar = None
-            elif newvar in varlist:
+            elif newvar.lower() in varlist:
                 newvar = None
             # end if
         # end while

scripts/suite_objects.py

@@ -1632,6 +1632,38 @@ def manage_variable(self, newvar):
             raise CCPPError(emsg)
         # end if
 
+    def ensure_unique_local_names(self):
+        """Rename any group-local variable (this Group's dictionary) whose
+        local name is also the local name of one of this Group's dummy
+        arguments (call list) or of a suite module variable.
+        The group subroutine declares its dummy arguments and its local
+        variables in the same scope, so a shared name is a Fortran error
+        (e.g., a constituent dummy argument that keeps the local name of the
+        scheme which first requested it while another scheme uses the same
+        local name for a scheme-supplied interstitial). A group local would
+        also shadow a same-named suite module variable, silently redirecting
+        any reference to that suite variable in this group's subroutine.
+        Renaming a group local is safe because generated code looks
+        variables up by standard name at write time and group locals are
+        invisible outside the group subroutine."""
+        reserved = self.call_list.known_local_names()
+        reserved.extend(self.parent.known_local_names())
+        rset = set(reserved)
+        for var in list(self.variable_list()):
+            lname = var.get_prop_value('local_name')
+            if lname.lower() in rset:
+                stdname = var.get_prop_value('standard_name')
+                new_lname = self.new_internal_variable_name(prefix=lname,
+                                                            reserved=reserved)
+                self.remove_variable(stdname)
+                # Add directly to this Group's dictionary (bypassing the
+                # SuiteObject dummy-argument adjustments already applied to
+                # the original variable)
+                VarDictionary.add_variable(self, var.clone(new_lname),
+                                           self.run_env, exists_ok=True)
+            # end if
+        # end for
+
     def analyze(self, phase, suite_vars, scheme_library, ddt_library,
                 check_suite_state, set_suite_state):
         """Analyze the Group's interface to prepare for writing"""
@@ -1653,6 +1685,9 @@ def analyze(self, phase, suite_vars, scheme_library, ddt_library,
         # end for
         self._phase_check_stmts = check_suite_state
         self._set_state = set_suite_state
+        # All of this Group's dummy arguments and local variables are now
+        # known; resolve any local name collisions between them
+        self.ensure_unique_local_names()
         if (self.run_env.logger and
             self.run_env.logger.isEnabledFor(logging.DEBUG)):
             self.run_env.logger.debug("{}".format(self))
@@ -1736,8 +1771,17 @@ def write(self, outfile, host_arglist, indent, const_mod,
             for var in item.declarations():
                 lname = var.get_prop_value('local_name')
                 sname = var.get_prop_value('standard_name')
-                if (lname in subpart_allocate_vars) or (lname in subpart_optional_vars) or (lname in subpart_scalar_vars):
-                    if subpart_allocate_vars[lname][0].compatible(var, self.run_env):
+                if lname in subpart_allocate_vars:
+                    prev_var = subpart_allocate_vars[lname][0]
+                elif lname in subpart_optional_vars:
+                    prev_var = subpart_optional_vars[lname][0]
+                elif lname in subpart_scalar_vars:
+                    prev_var = subpart_scalar_vars[lname][0]
+                else:
+                    prev_var = None
+                # end if
+                if prev_var is not None:
+                    if prev_var.compatible(var, self.run_env):
                         pass # We already are going to declare this variable
                     else:
                         errmsg = "Duplicate Group variable, {}"

test/advection_test/CMakeLists.txt

@@ -1,5 +1,5 @@
 # Create list of SCHEME_FILES, HOST_FILES, and SUITE_FILES
-set(SCHEME_FILES       "cld_liq" "cld_ice" "apply_constituent_tendencies" "const_indices")
+set(SCHEME_FILES       "cld_liq" "cld_ice" "apply_constituent_tendencies" "const_indices" "cld_shadow")
 set(SCHEME_FILES_ERROR "cld_liq" "cld_ice" "dlc_liq")
 set(HOST_FILES         "test_host_data" "test_host_mod")
 set(SUITE_FILES        "cld_suite.xml")

test/advection_test/README.md

@@ -8,6 +8,9 @@ Contains tests to exercise the capabilities of the constituents object, includin
 - Accessing and modifying a constituent tendency variable
 - Passing around the constituent tendency array
 - Dimensions are case-insensitive
+- Reuse of a constituent or host variable local name by a scheme
+  interstitial with a different standard name (cld_shadow); the group cap
+  must rename its local variables to avoid duplicate declarations
 
 ## Building/Running
 

test/advection_test/advection_test_reports.py

@@ -35,7 +35,8 @@
 _CCPP_FILES = _UTILITY_FILES + _HOST_FILES + _SUITE_FILES
 _DEPENDENCIES = [""]
 _PROCESS_LIST = [""]
-_MODULE_LIST = ["cld_ice", "cld_liq", "const_indices", "apply_constituent_tendencies"]
+_MODULE_LIST = ["cld_ice", "cld_liq", "cld_shadow", "const_indices",
+                "apply_constituent_tendencies"]
 _SUITE_LIST = ["cld_suite"]
 _REQUIRED_VARS_CLD = ["ccpp_error_code", "ccpp_error_message",
                       "horizontal_loop_begin", "horizontal_loop_end",

test/advection_test/cld_shadow.F90

@@ -0,0 +1,41 @@
+! Test parameterization whose local names collide with other identifiers
+! in the group cap: <cld_ice_array> is also the local name of the cloud
+! ice constituent (see cld_ice) and <ncols> is also the local name of the
+! host model horizontal dimension. Both name a scheme-supplied
+! interstitial here, so capgen must rename the group-cap locals.
+!
+
+module cld_shadow
+
+  use ccpp_kinds, only: kind_phys
+
+  implicit none
+  private
+
+  public :: cld_shadow_run
+
+contains
+
+  !> \section arg_table_cld_shadow_run  Argument Table
+  !! \htmlinclude arg_table_cld_shadow_run.html
+  !!
+  subroutine cld_shadow_run(ncol, timestep, cld_ice_array, ncols,       &
+       errmsg, errflg)
+
+    integer,            intent(in)  :: ncol
+    real(kind_phys),    intent(in)  :: timestep
+    real(kind_phys),    intent(out) :: cld_ice_array(:,:)
+    real(kind_phys),    intent(out) :: ncols(:)
+    character(len=512), intent(out) :: errmsg
+    integer,            intent(out) :: errflg
+    !----------------------------------------------------------------
+
+    errmsg = ''
+    errflg = 0
+
+    cld_ice_array(:ncol,:) = timestep
+    ncols(:ncol) = real(ncol, kind_phys)
+
+  end subroutine cld_shadow_run
+
+end module cld_shadow

test/advection_test/cld_shadow.meta

@@ -0,0 +1,46 @@
+[ccpp-table-properties]
+  name = cld_shadow
+  type = scheme
+[ccpp-arg-table]
+  name = cld_shadow_run
+  type = scheme
+[ ncol ]
+  standard_name = horizontal_loop_extent
+  type = integer
+  units = count
+  dimensions = ()
+  intent = in
+[ timestep ]
+  standard_name = time_step_for_physics
+  long_name = time step
+  units = s
+  dimensions = ()
+  type = real | kind = kind_phys
+  intent = in
+[ cld_ice_array ]
+  standard_name = cld_shadow_scratch_array
+  units = s
+  type = real | kind = kind_phys
+  dimensions = (horizontal_loop_extent, vertical_layer_dimension)
+  intent = out
+[ ncols ]
+  standard_name = cld_shadow_column_scratch
+  units = count
+  type = real | kind = kind_phys
+  dimensions = (horizontal_loop_extent)
+  intent = out
+[ errmsg ]
+  standard_name = ccpp_error_message
+  long_name = Error message for error handling in CCPP
+  units = none
+  dimensions = ()
+  type = character
+  kind = len=512
+  intent = out
+[ errflg ]
+  standard_name = ccpp_error_code
+  long_name = Error flag for error handling in CCPP
+  units = 1
+  dimensions = ()
+  type = integer
+  intent = out

test/advection_test/cld_suite.xml

@@ -7,5 +7,6 @@
     <scheme>apply_constituent_tendencies</scheme>
     <scheme>cld_ice</scheme>
     <scheme>apply_constituent_tendencies</scheme>
+    <scheme>cld_shadow</scheme>
   </group>
 </suite>