[flang-commits] [flang] [flang][Semantics] Introduce `-Wpass-global-variable` warning (PR #160324)

[Andre Kuhlenschmidt via flang-commits]

================
@@ -56,9 +56,64 @@ static void CheckImplicitInterfaceArg(evaluate::ActualArgument &arg,
         "%VAL argument must be a scalar numeric or logical expression"_err_en_US);
   }
   if (const auto *expr{arg.UnwrapExpr()}) {
-    if (const Symbol * base{GetFirstSymbol(*expr)};
-        base && IsFunctionResult(*base)) {
-      context.NoteDefinedSymbol(*base);
+    if (const Symbol * base{GetFirstSymbol(*expr)}) {
+      if (IsFunctionResult(*base)) {
+        context.NoteDefinedSymbol(*base);
+      } else {
+        // passing global variables
+        // here, arrays with subscripts are processing
+        bool warn{false};
+        std::string ownerName{""};
+        std::string ownerType{""};
+        if (base->flags().test(Symbol::Flag::InCommonBlock)) {
+          const Symbol *common{FindCommonBlockContaining(*base)};
+          ownerType = "COMMON";
+          ownerName = common->name().ToString();
+          if (!(base->Rank() == 1 && base->offset() == 0)) {
+            warn |= true;
+          } else if (base->Rank() == 1) {
+            if (const ArraySpec *dims{base->GetShape()};
+                dims && dims->IsExplicitShape()) {
+              if (!((*dims)[0].lbound().GetExplicit() == (*dims)[0].ubound().GetExplicit())) {
+                warn |= true;
+              }
+            }
+            if (common->get<CommonBlockDetails>().objects().size() > 1) {
+              warn |= true;
+            }
+          }
+        } else if (const auto &owner{base->GetUltimate().owner()};
+                   owner.IsModule() || owner.IsSubmodule()) {
+          const Scope *module{FindModuleContaining(owner)};
+          ownerType = "MODULE";
+          ownerName = module->GetName()->ToString();
+          if (base->attrs().test(Attr::PARAMETER)) {
+            warn |= false;
+          } else if (base->Rank() != 1) {
+            warn |= true;
+          } else if (!base->attrs().test(Attr::ALLOCATABLE) &&
+                     !base->attrs().test(Attr::POINTER) &&
+                     !base->attrs().test(Attr::VOLATILE)) {
+            // by some reason, dims is not constructed here. For common blocks' variables, it works
+            // it leads to three skipped tests
+            /*
+            if (const ArraySpec *dims{base->GetShape()};
+                dims && dims->IsExplicitShape()) {
+              if (!((*dims)[0].lbound().GetExplicit() == (*dims)[0].ubound().GetExplicit())) {
+                  warn |= true;
+              }
+            }
----------------
akuhlens wrote:

https://github.com/llvm/llvm-project/blob/d79036c1aa73bc99183742926977a5cb9823b97b/flang/lib/Semantics/symbol.cpp#L453

This could be found by searching for `GetShape path:flang path:symbol` within the repo. I use path:symbol because there were too many entries when I searched without it, and you were calling GetShape which seems to be a method of Symbols.

Based on the code, it returns nullptr when the symbol doesn't refer to a "object entity". I think this corresponds to "data entity" in the [fortran '23 standard](https://j3-fortran.org/doc/year/24/24-007.pdf) 5.4.3, which roughly corresponds to the notion of a value in general programming language theory (I think it specifically refers to the reference to a value). Not sure how familiar you are with fortran, but Fortran is weird in that the type system (and thus values) doesn't actually cover all "entities" that can be referenced in the syntax of the language. For instance functions can be referenced via symbols, but are not actually values (data entities) only their results are. 

https://github.com/llvm/llvm-project/pull/160324