[flang-commits] [flang] [flang] Lower PRIVATE component names safely (PR #66076)

[via flang-commits]

jeanPerier wrote:

> This change will make IR harder to read.

For private components, yes. It is a trade off I chose over more complex compiler code and more time to find the name of "normal" components. It is an easy rule to apply.
 
>  Is it possible to have a 3-way or a 4-way component name conflict? (If this is limited to a 2-way conflict, then it might be better to mangle the (unique?) child name.)

You can have any number of components with the same name:
```
module m1
 type t1
   integer, private :: i
 end type
end module
module m2
 use m1
 type, extends(t1) ::  t2
   integer, private :: i
 end type
end module
module m3
 use m2
 type, extends(t2) :: t3
   integer, private :: i
 end type
end module
! .....
```

> If not, it seems like it would be better to mangle child names.

>From an IR readability, I agree, the problem with this approach is that every time you want to know a field name you will then need a linear search over the other type components to look for conflict.

> Would it be possible to solve the problem by always adding a prefix or suffix tag to private components?
 
No, see example above, you can have any number of component conflicting.
 
> Or can some kind of numeric "extension depth" tag be used to a differentiate names and avoid conflicts?

Yes but at compile time cost for a lot of regular types with no private components.

> That's also kind of heavy weight, but the cost is compile time rather than changing the IR.

What I do not like is that the cost of this analysis will be paid by every derived type, even if they do not have private components (since you cannot know that until you have looked at all the other components).

> If nothing else helps to avoid adding type name tags, I think it would be better to swap the order and put the containing type name first.

There is a "cheaper" approach that would keep the IR lighter, but I am not a big fan. It is to mangle private components _only_ when they appear in an extension of the base type containing the component. There is no extra cost for non private component (protected by a simple Private attribute check). However, this means the private components will be mangled differently in the base and extended type, so it makes it harder to make "connections" between the components of a type and the components of this extensions... (let say risky, people would mostly not think of this case an just assume the name must be the same). You would basically have `!fir.type<_QMmodTt1{i:i32}>` and !fir.type<_QMmod2Tt2{i._QMmodTt1:i32, j:i32}>>`.


My take from our tests is that private components are not so common so that it is worth increasing complexity for the rest and it is not worth breaking the easy to work with predicate that component of some base type always have the same FIR field name in the type extensions.

Regarding the ugliness of the IR, we have a long due task to use MLIR type alias to improve the IR readability (the full !_QMsomemodTsometype =  fir.type<.....> would only be defined once and then you would directly see  fir.ref<!_QMsomemodTsometype> in the IR without the whole list of components every time the types appear).

Would you be OK with the current approach given the possibility of mlir type aliasing?

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