New version to work with 'common' statements

[schreiberx]

This is an alternative version to work with 'common' statements.
Here, common statements are directly supported in the symbol table rather than using workarounds.

[codecov]

Codecov Report

❌ Patch coverage is 96.03175% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.94%. Comparing base (5f5004c) to head (58d0bef).

Files with missing lines	Patch %	Lines
src/psyclone/psyir/symbols/symbol_table.py	87.17%	5 Missing ⚠️
src/psyclone/psyir/symbols/symbol.py	81.81%	4 Missing ⚠️
src/psyclone/psyir/symbols/interfaces.py	95.00%	1 Missing ⚠️

❌ Your project check has failed because you have indirect coverage changes. Learn more about Unexpected Coverage Changes and reasons for indirect coverage changes.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3444      +/-   ##
==========================================
- Coverage   99.96%   99.94%   -0.02%     
==========================================
  Files         391      392       +1     
  Lines       54689    54898     +209     
==========================================
+ Hits        54670    54870     +200     
- Misses         19       28       +9     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[arporter]

Thanks @schreiberx. Before looking very closely at this, I just want to discuss something: this PR adds the concept of a CommonBlockSymbol which is treated in the same way as all other Symbols - i.e. it shares their namespace. This is not the way Fortran works, e.g.:

subroutine my_sub()
  common /andy/ b, c
  integer :: b, c
  integer :: andy
end subroutine my_sub

compiles fine. However, the question is whether @sergisiso, @AidanChalk or @hiker think that this sharing of the namespace actually matters? It will require some special handling when e.g. merging symbol tables (e.g. to ignore a clash if one of the symbols is a CommonBlockSymbol) but are there any other bigger issues that people can see?

[sergisiso]

We avoided adding this Symbol type the first time for this same reason. And I still think it should not be a symbol, it matches better with the psyir Interface concept. For me:

  common /andy/ b, c
  integer :: b, c

Should be treated similarly to:

  save b, c
  integer :: b, c

• Both statements (depending which comes first) create symbols b and c.
• The first statement sets the interface for this symbols, StaticInterface (for save) or a new CommonBlockInterface('andy'), with the common block name inside as a string, not a symbol.
• The second statement sets the type of the symbol (but not the interface if these symbols already exist)

[sergisiso]

oh, it already works like this:

But we keep a PSYCLONE_INTERNAL_COMMONBLOCK symbol because this can not be inserted in the delcaration like save, so this encodes the "common block statement".

What problem are you trying to solve here @schreiberx ?

[sergisiso]

Sorry I am catching up slowly, but I agree the solution is @arporter comment here: #3389 (review)

[arporter]

I like the interface idea Sergi: it would then quite naturally work like module USE statements - we'd examine all Symbols and create COMMON declarations for all of the distinct COMMON blocks that they are associated with.

[schreiberx]

oh, it already works like this: But we keep a PSYCLONE_INTERNAL_COMMONBLOCK symbol because this can not be inserted in the delcaration like save, so this encodes the "common block statement".

What problem are you trying to solve here @schreiberx ?

The alternative approach (another PR) used regular expressions to search for this. It's simply to avoid this.

[sergisiso]

Thanks @schreiberx as discussed in Teams I would prefer to continue the other branch rather than this one