eclipse / omr

Eclipse OMR™ Cross platform components for building reliable, high performance language runtimes
http://www.eclipse.org/omr
Other
934 stars 392 forks source link

Figure out name collision avoidance strategy #1706

Open lmaisons opened 6 years ago

lmaisons commented 6 years ago

As part of our refactoring work for releasing the platform independent code to the OMR project, the compiler component used the OMR namespace to hold its runtime-agnostic types. More recently, there has (apparently) been a discussion for the GC to begin using the OMR namespace as well. That has started to appear in concrete form (for example in #1691).

Conceptually it makes just as much sense for the GC to use the OMR namespace as the compiler, with primacy being the only counter-argument I can conceive of.

Given that we probably do not want to give primacy arguments any quarter, we should thus proceed to figure out how we want to resolve the (potentially significant) increase in the probability of identifiers colliding.

charliegracie commented 6 years ago

This is easily solved in my opinion. I originally suggested that ALL OMR namespaces should be directly followed by a component. This way we can define the list of components and then each component is free to give whatever name it chooses to classes, etc without worrying about any namespace collisions.

Examples that I believe should be fine. Please correct me if I am wrong. OMR::GC:: OMR::COMPILER::

Things we should NOT do in my opinion: OMR::TR:: <--- the TR namespace is a J9 thing and not an OMR thing. We should move away from this. OMR::J9GC:: <--- bad for the same reasons as the TR namespace

mgaudet commented 6 years ago

In the compiler, the TR:: namespace is not project specific; it is used to refer to concrete concepts

charliegracie commented 6 years ago

I understand what is was used for but the TR:: namespace is very confusing when you start to look into OpenJ9 codebase. Now that both projects are open I would really like to see the J9 and TR names disappear from the OMR codebase as much as possible. I think the for the consumption / adoption of OMR by other projects it has to be clean of all of the OpenJ9'ism.

Leonardo2718 commented 6 years ago

The subtlety of the TR:: namespace is that, whatever its name is, it must be the same in all direct consumers of the OMR compiler. So if we change it to something else in OMR, it will have to also be changed in OpenJ9 as well as any other downstream project of the compiler technology.

0xdaryl commented 6 years ago

That's correct. TR:: does pay homage to the Testarossa compiler technology which has been used to compile more than just Java in the past. But classes in the TR:: namespace are an essential part of the extensible class framework used throughout OMR and projects that consume OMR. As @mgaudet mentioned, it is used to refer to the most-derived, concrete class that is formed through composition from OMR and any downstream project extension. It is essential to have a namespace where that concrete class can be referred to consistently, regardless of which "project" (OMR, OpenJ9, Ruby, Lua, JitBuilder, etc.) you're in.

If it isn't TR:: then it will need some other name. Before contributing the compiler technology we debated the best possible name that was semantically appealing and convenient to type, and that was the best choice.

charliegracie commented 6 years ago

I will concede that changing the 'TR::' namespace would be a significant change at this point so maybe it is not worth it. It would be nicer if it was clear that it is the name of the concrete impls for all of the OMR compiler classes but I guess that decision is long past.

Now back to the conversation at hand since I seemed to have derailed it a little. I think OMR as a top level namespace is the only thing that makes sense. Each component should then have its own namespace as well. I would argue deeper levels of namespaces but that is a component decision in my opinion. What does everyone think about the OMR namespace for the project followed by component namespaces?

Examples are similar to above: OMR::GC OMR::COMPILER OMR::RAS ..... etc.

0xdaryl commented 6 years ago

For nested namespaces that are names rather than acronyms my eye prefers lowercase than the acoustically displeasing uppercase (e.g., OMR::Compiler, OMR::Port, OMR::GC, ...)

Compiler is a bit long but I don't have a better suggestion (and I don't want to abbreviate it). The compiler technology also already uses nested namespaces for architectures and subarchitectures (e.g., OMR::X86::i386::TreeEvaluator which would now become OMR::Compiler::X86::i386::TreeEvaluator). Its a bit long, but tolerable in my opinion.

charliegracie commented 6 years ago

Ok great @0xdaryl I think that makes a lot of sense. I would be ok with us starting to use these namespaces. Once @rwy0717 has completed the changes to his PR I will accept it with the OMR::GC:: namespace.

@mstoodle, @vijaysun-omr, @jduimovich and @youngar can I get a thumbs up from you on this subject if you agree? Thanks

vijaysun-omr commented 6 years ago

I agree with the last comment from @0xdaryl on nested namespaces.

charliegracie commented 6 years ago

I feel like this issue has been settled and agreed upon. If so can we close it?

rwy7 commented 6 years ago

I think you're right @charliegracie, but there is one more thing I'd like to bring up.

One day, I'd like to see code dealing with cross-cutting concerns move to a new common component (or multiple components). If that happens, what namespace would we use? I think I'm going to open an issue about improving code sharing, and we can answer this question there?

0xdaryl commented 6 years ago

Perhaps a related question is whether any code would ever live in OMR:: without a nested namespace? I think the answer is yes, and cross component features like @rwy0717 mentions fit the bill in my opinion.