[flang-commits] [PATCH] D124622: [flang] Handle common block with different sizes in same file

[Jean Perier via Phabricator via flang-commits]

jeanPerier marked 3 inline comments as done.
jeanPerier added a comment.

> Could you add comments to docs/Extensions.md to describe that (1) initializations of common outside BLOCK DATA and (2) initializations of blank COMMON are both non-standard, but are near-universal extensions that we have to support?

Thanks for the reminder. (2) was already describe there, I added (1) right next to it.



================
Comment at: flang/include/flang/Lower/PFTBuilder.h:32
+namespace Fortran::semantics {
+using CommonBlockList = std::vector<std::pair<SymbolRef, std::size_t>>;
+}
----------------
klausler wrote:
> It seems a little risky to define this typedef in two distinct header files; what if one of them changes?
In this specific case, both definitions are visible in PFTBuilder.cpp file, so if there is a conflict, the C++ compiler (at least clang) would raise "error: type alias redefinition with different types".

But I agree it is more maintenance to maintain two definition, I included semantics.h. It is not too template heavy, so there is little point avoiding to include it here.


================
Comment at: flang/include/flang/Semantics/semantics.h:208
+  // This can obviously not check any conflicts between different compilation
+  // units (in case such conflicts exist, the behavior will depend of the
+  // linker).
----------------
klausler wrote:
> "depend on" or "depend upon"
Thanks


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D124622/new/

https://reviews.llvm.org/D124622