Clean up P4C namespaces

[fruffy]

Our namespace policy is a little unclear. Currently, we seem to use P4 for front/mid end, ControlPlaneAPI for the control plane, and IR for the IR code. The lib folder has no namespaces.

This namespace policy makes it more difficult to link P4C as a library and also introduce elements that can be used across the entire compiler framework without polluting the global namespace.

It might be worthwhile to step back and figure out a clearer policy on namespaces. The difficulty is that any changes to the core code will be a major breaking change.

[qobilidop]

I'm starting to use P4C as a library at work, and this has come up as a major issue.

In the short term, in order to minimize downstream code changes, I wonder if it makes sense to introduce an extra top-level namespace P4C (personally I'd prefer to use p4c, but P4C seems more consistent with existing convention)? Current namespaces will become nested namespaces like P4C::IR. The benefit is that any existing code can do a one-line using namespace P4C; change, and expect the rest of the code to work.

After this refactoring, I guess figuring out a clearer namespace policy can be left as a longer term issue to solve over time.

[ChrisDodd]

Currently, most things in the compiler should be in one of a few namespaces:

• P4 for frontend/midend passes
• IR for IR related stuff
• Util for general utilities
• a backend-specific namespace

[fruffy]

I'm starting to use P4C as a library at work, and this has come up as a major issue.

In the short term, in order to minimize downstream code changes, I wonder if it makes sense to introduce an extra top-level namespace P4C (personally I'd prefer to use p4c, but P4C seems more consistent with existing convention)? Current namespaces will become nested namespaces like P4C::IR. The benefit is that any existing code can do a one-line using namespace P4C; change, and expect the rest of the code to work.

After this refactoring, I guess figuring out a clearer namespace policy can be left as a longer term issue to solve over time.

We could make a PR to get started with this and then see what breaks etc.

[qobilidop]

Sure, I can draft a PR for this.

[qobilidop]

Just want to have some further discussion regarding #4825 and follow-ups.

My goal for #4825 is to serve as a good starting point for further clean-ups. The diffs are intentionally kept as small and straightforward as possible, leaving a lot of room for further improvements.

So after #4825 is merged, maybe we could do one round of more careful review & refactoring on a per-subdirectory basis? If there are a few of us available for this work, I propose that we coordinate in advance who will review which subdirectory, so we can approach this in parallel, and don't double the work by accident.

What does everyone think?

[fruffy]

What does everyone think?

Any concrete cleanups you have in mind? I can do this ad-hoc for P4Tools at the very least.

[qobilidop]

Any concrete cleanups you have in mind? I can do this ad-hoc for P4Tools at the very least.

Some concrete items I can think of now:

• Eliminate the use of using namespace P4.
• Check if things are defined in the appropriate namespace. For example, I've previously noticed that in some backend code, there were things accidentally defined in the global namespace (now under ::P4), but should be moved to the corresponding backend namespace.
• Some macros are currently defined within a namespace block. They should probably be moved out to the global namespace.
• In some macros the P4:: prefix is missing which might cause usage issues. @vlstill has already caught several such cases. For the remaining ones, we can probably fix them in these follow-up PRs.

In general, I feel that once we start going over each directory more carefully, we might be able to identify more namespace issues or room for further cleanups.

Also, I can sign up to review the following directories that I feel I have a relatively good understanding of:

• backends
  • bmv2
  • common
  • graphs
  • p4fmt
  • p4test
• test

[qobilidop]

Reopen as this issue is not fully resolved. Keep it to track further namespace cleanups.

[fruffy]

Reopen as this issue is not fully resolved. Keep it to track further namespace cleanups.

Feel free to edit the original post to create an umbrella issue or create an entirely new one.

[vlstill]

Please update https://github.com/p4lang/p4c/blob/main/.gdbinit to call dump/dbprint/... from the new namespace.

[qobilidop]

Reopen as this issue is not fully resolved. Keep it to track further namespace cleanups.

Feel free to edit the original post to create an umbrella issue or create an entirely new one.

Created #4931 for this.

I think the original question of having a clearer policy on namespaces is still not addressed yet. Maybe discussion on that can continue here. Adding a doc (or a section in an existing doc) to explain P4C's namespace policy might be a realistic action item to close this issue.