apple reviewer feedback

Signed-off-by: Jade Abraham <[email protected]>

Author: jabraham17

util/chplenv/chpl_atomics.py

@@ -82,8 +82,9 @@ def get(flag='target'):
             # we can't use intrinsics, fall back to locks
             if not atomics_val:
                 atomics_val = 'locks'
-        else:
-            check_valid_var("CHPL_ATOMICS", atomics_val, ['cstdlib', 'intrinsics', 'locks'])
+
+        check_valid_var("CHPL_ATOMICS", atomics_val, ['cstdlib', 'intrinsics', 'locks'])
+
     else:
         error("Invalid flag: '{0}'".format(flag), ValueError)
 

util/chplenv/chpl_comm_ofi_oob.py

@@ -41,32 +41,30 @@ def get():
         return 'none'
 
     oob_val = overrides.get('CHPL_COMM_OFI_OOB')
-    if oob_val:
-        check_valid_var("CHPL_COMM_OFI_OOB", oob_val, ("mpi", "pmi2", "sockets"))
-        return oob_val
-
-    #
-    # By default, use PMI2 out-of-band support on Cray X* and HPE Cray EX
-    # systems, MPI on other Cray systems or with an MPI-based launcher,
-    # and "sockets" otherwise.
-    #
-    platform_val = chpl_platform.get('target')
-    launcher_val = chpl_launcher.get()
-    if 'cray-x' in platform_val or chpl_platform.is_hpe_cray('target'):
-        oob_val = 'pmi2'
-    elif 'cray-' in platform_val:
-        oob_val = 'mpi'
-    elif 'mpi' in launcher_val:
-        oob_val = 'mpi'
-    else:
-        import chpl_compiler
-        if _find_pmi2() is not None:
-            oob_val = 'pmi2'
-        elif "-lpmi2" in chpl_compiler.get_system_link_args('target'):
+    if not oob_val:
+        #
+        # By default, use PMI2 out-of-band support on Cray X* and HPE Cray EX
+        # systems, MPI on other Cray systems or with an MPI-based launcher,
+        # and "sockets" otherwise.
+        #
+        platform_val = chpl_platform.get('target')
+        launcher_val = chpl_launcher.get()
+        if 'cray-x' in platform_val or chpl_platform.is_hpe_cray('target'):
             oob_val = 'pmi2'
+        elif 'cray-' in platform_val:
+            oob_val = 'mpi'
+        elif 'mpi' in launcher_val:
+            oob_val = 'mpi'
         else:
-            oob_val = 'sockets'
-
+            import chpl_compiler
+            if _find_pmi2() is not None:
+                oob_val = 'pmi2'
+            elif "-lpmi2" in chpl_compiler.get_system_link_args('target'):
+                oob_val = 'pmi2'
+            else:
+                oob_val = 'sockets'
+
+    check_valid_var("CHPL_COMM_OFI_OOB", oob_val, ("mpi", "pmi2", "sockets"))
     return oob_val
 
 # returns 2-tuple of lists

util/chplenv/chpl_comm_segment.py

@@ -21,8 +21,7 @@ def get():
                 segment_val = 'large'
             else:
                 segment_val = 'everything'
-        else:
-            check_valid_var("CHPL_GASNET_SEGMENT", segment_val, ("none", "fast", "large", "everything"))
+        check_valid_var("CHPL_GASNET_SEGMENT", segment_val, ("fast", "large", "everything"))
     else:
         segment_val = 'none'
     return segment_val

util/chplenv/chpl_jemalloc.py

@@ -53,13 +53,13 @@ def get(flag='target'):
         elif darwin and mem_val == 'jemalloc' and jemalloc_val == 'bundled':
             error("CHPL_HOST_JEMALLOC=bundled is not supported on Mac for host builds")
 
+    var_name = 'CHPL_{0}_JEMALLOC'.format(flag.upper())
+    mem_var_name = 'CHPL_{0}_MEM'.format(flag.upper())
     if mem_val == 'jemalloc' and jemalloc_val == 'none':
-        error("CHPL_JEMALLOC must not be 'none' when CHPL_TARGET_MEM is jemalloc")
+        error("{0} must not be 'none' when {0} is jemalloc".format(var_name, mem_var_name))
     elif mem_val != 'jemalloc' and jemalloc_val != 'none':
-        error("CHPL_JEMALLOC must be 'none' when CHPL_TARGET_MEM is not jemalloc")
+        error("{0} must be 'none' when {0} is not jemalloc".format(var_name, mem_var_name))
 
-    var_name = 'CHPL_{0}_JEMALLOC'.format(flag.upper())
-    var_name = 'CHPL_JEMALLOC' if chpl_target_jemalloc is None else var_name
     check_valid_var(var_name, jemalloc_val, ["none", "bundled", "system"])
     return jemalloc_val
 

util/chplenv/chpl_libfabric.py

@@ -18,12 +18,11 @@ def get():
                 libfabric_val = 'system'
             else:
                 libfabric_val = 'bundled'
-        if libfabric_val == 'none':
-            error("CHPL_LIBFABRIC must not be 'none' when CHPL_COMM is ofi")
-        elif chpl_platform.is_hpe_cray('target') and libfabric_val != 'system':
-            warning('CHPL_LIBFABRIC!=system is discouraged on HPE Cray')
 
         check_valid_var('CHPL_LIBFABRIC', libfabric_val, ['bundled', 'system'])
+        if chpl_platform.is_hpe_cray('target') and libfabric_val != 'system':
+            warning('CHPL_LIBFABRIC!=system is discouraged on HPE Cray')
+
     else:
         libfabric_val = 'none'
 

util/chplenv/chpl_locale_model.py

@@ -8,7 +8,7 @@
 @memoize
 def get():
     locale_model_val = overrides.get('CHPL_LOCALE_MODEL', 'flat')
-    check_valid_var("CHPL_LOCALE_MODEL", locale_model_val, ["flat", "shared"])
+    check_valid_var("CHPL_LOCALE_MODEL", locale_model_val, ["flat", "gpu"])
     return locale_model_val
 
 

util/chplenv/chpl_mem.py

@@ -39,7 +39,6 @@ def get(flag='host'):
         error("Invalid flag: '{0}'".format(flag), ValueError)
 
     var_name = 'CHPL_{0}_MEM'.format(flag.upper())
-    var_name = 'CHPL_MEM' if chpl_target_mem is None else var_name
     check_valid_var(var_name, mem_val, ["cstdlib", "jemalloc"])
     return mem_val
 

util/chplenv/utils.py

@@ -182,7 +182,13 @@ def is_ver_in_range(versionStr, minimumStr, maximumStr):
     return False
 
 def check_valid_var(varname, value, valid_values):
-    """Check that a variable is set to a valid value"""
+    """
+    Check that a variable is set to a valid value
+
+    :param varname: The name of the variable, used for printing the error message
+    :param value: The value of the variable as set by the user or as inferred
+    :param valid_values: A sequence of valid values for the variable, e.g. a tuple or a list
+    """
 
     def join_words(words, conjunction='or'):
         if len(words) == 1: