Omr: Revisit philosophy and use of the TR:: Namespace

[Leonardo2718]

3287 has raised some questions about whether non-extensible classes in the OMR compiler component should go in the TR namespace or the OMR namespace. Although a discussion that happened over a year ago provides some guidance for this, the lack of consensus on the topic now suggests that further clarification is needed and a potential change of guideline may be warranted.

To summarize various issues related to the topic:

I believe the original discussion about when to use TR:: vs. OMR:: began in #788. The conclusion resulted in #817 which added the following paragraph in Extensible_Classes_in_OMR.md:

In the case of classes that aren't extensible, It is perfectly legitimate to create a class directly in the TR:: namespace if it's complete and can and should be used as-is.

The paragraph can be interpreted to mean that non-extensible classes should go in the TR namespace (although that's not strictly what it says). A follow up issue like #1650 seems to support this interpretation.

Although not directly related, #1706 and #1885 discuss using component-level namespaces and moving class in the compiler component from OMR:: to OMR::Compiler::. The issues don't address what goes and doesn't go in the TR namespace but do mention some subtleties of how the namespace is used.

Finally, #2084 proposes to clarify "proper use" of the TR namespace and to move the documentation outside of Extensible_Classes_in_OMR.md, since the namespace is also used for non-extensible classes.

Recent discussions, including #3287, have put into question whether the guideline to put non-extensible classes in the TR namespace is appropriate. This issue aims to track people's current thoughts on the topic and to develop more complete and precise guidelines for the use of TR::.

A non-executive list of points to consider:

• what are the implications of using the TR vs. OMR (vs. OMR::Compiler) namespaces for non-extensible classes in downstream projects?
• what work will have to be done to move existing non-extensible classes from TR:: to OMR:: (or OMR::Compiler::?
• what should happen with classes that use the TR_ prefix in stead of TR:: (a convention used in the compiler component long before it was opensourced)?

FYI @0xdaryl @mstoodle @fjeremic @vijaysun-omr

[mstoodle]

I can see both sides of this one :( .

On the one hand, it seems like having all the compiler classes one would consume being in one namespace would mean client projects won't have to worry about whether a class is extensible or not, and if an earlier non-extensible class becomes extensible that wouldn't break projects that have references to the non-extensible class.

On the other hand, extensible classes are "different" than non-extensible classes in that they not only refer to OMR code but the fully specialized code (including OMR as well as any project specializations). Using an extensible class isn't just using an OMR implementation (but may be). The breakage induced in consuming projects by making a class extensible would provide a timely red flag to a consuming project that there's something to think about (i.e. do they need to specialize or not). But I wonder if there could still be silent failures, though, because consuming project would be directly referencing the OMR layer of an extensible class (gulp and grimace).

That would seem to argue in favour of putting everything into TR namespace...

[fjeremic]

The breakage induced in consuming projects by making a class extensible would provide a timely red flag to a consuming project that there's something to think about (i.e. do they need to specialize or not). But I wonder if there could still be silent failures, though, because consuming project would be directly referencing the OMR layer of an extensible class (gulp and grimace).

I too can envision this being a common source of issues.

[samolisov]

My 5 rubles. Almost all optimizations and support classes (TR_AsyncCheckInsertion, TR_CallStack, TR_CFGSimplifier, TR_CompactLocals, TR_BitVector etc.) are out of any namespace and started with the TR_ prefix. As I see, the optimizations aren't marked as extensible except the basic class TR::Optimization itself. Maybe the classes also have to be in a namespace: OMR or TR? For example, in LLVM all passes not-touched from other passes live in the anonymous namespace but if a pass is available from another one (llvm/Analysis/AliasAnalysis.h), the pass lives directly in the llvm namespace.

[Leonardo2718]

@samolisov Yes, you're right. The TR_ prefix is vestigial and should be removed in favour of using a namespace. We were working on this at one point but that work has kind of died down as other priorities surfaced. :disappointed:

[Leonardo2718]

To summarize the discussion that took place during the #3326 Compiler Architecture Meeting, the following decision was made:

All parts of the OMR compiler's public API go in the TR namespace.

The justifications for this decision are as follows:

• The TR namespace must be used for extensible classes. Putting non-extensible classes and other utilities in the TR namespace as well ensures the OMR compiler's API is unified under one namespace and avoids the confusion of having some parts in TR and other parts in a different namespace.
• Putting all the OMR compiler classes in the TR namespace can minimizes breakage for users when an API changes. For example, if a non-extensible class is made into an extensible class, users of the class don't need to change what namespace they use to access the class. However, if a different namespace is used for non-extensible classes, all uses of the namespace to access the class have to be updated. Furthermore, if the non-extensible class used to reside in the OMR namespace (the namespace that is used for the implementation of extensible class components), forgetting to change the namespace would likely result in unexpected and mysterious failures; the typical problems of using a partially defined extensible class.
• While the possibility of moving from static to dynamic polymorphism for extensible classes is being investigated, it is not yet clear if/when it will be possible to eliminate the need for the TR namespace. It is therefore best to assume the TR namespace will continue to exist for the foreseeable future.

The next step to resolving this issue is to document the decision and justifications given above. The documentation should be general and not specific to extensible classes. It should also be easy to find and understandable by readers who are not already familiar with the topic.

The discussion for using OMR vs OMR::Compiler as a top-level namespace for the compiler component is not directly related to this specific issue, so I will only summarize some of the discussion points:

• Using OMR::Compiler would make the length of many declarations much longer than they currently are. The added length could lead to more widespread use of languages features such auto and using namespace declarations, which may make the code more brittle and difficult to maintain and understand.
• The main justification for using component specific namespaces inside the top-level OMR namespace is to avoid potential name conflicts between components. However, other large-scale projects, such as LLVM, are able to avoid name collisions while also only using one top-level namespace. Also, name conflicts are an indicator of opportunities for unifying similar functionality across components and improving the overall architecture of the project.

[0xdaryl]

The next step to resolving this issue is to document the decision and justifications given above. The documentation should be general and not specific to extensible classes. It should also be easy to find and understandable by readers who are not already familiar with the topic.

A PR is coming to modify the "Namespace" documentation in the compiler README.md. I don't think it is necessary to provide justification for the decision to use the TR namespace there. Rather, I believe that's what this issue is for.