Promote P4 to be THE project-wise top-level namespace

[qobilidop]

The context is #4707.

Since this refactoring touches almost every C++ (and some non-C++) file, I tried to make the diffs as minimal as possible, to reduce the possibility of breaking things by accident. As a result, there is still much room in improving coding styles.

Details of major changes applied:

• For existing top-level namespaces other than P4, like namespace IR, replace them globally to be namespace P4::IR.
• For symbols defined in the global namespace, move them into namespace P4 manually.
• For fully-qualified symbols like ::error, replace them globally to be ::P4::error.
• For non-header code, add using namespace ::P4; as we see fit.
• IR and parser generators are the trickier parts to deal with, as C++ code generations are involved.

The rest are minor changes to make the code compile without errors.

[qobilidop]

One thing to further discuss is whether we want the namespace to be p4c or P4C? I'm currently using p4c for my personal preference, but it's relatively easy to change to P4C.

My main reasons for preferring p4c are:

1. I'm personally used to the Google C++ Style Guide.
2. Outside Google, it seems the snake_case namespace naming style is also more common than CamelCase.
3. Sometimes the correct capitalization is not very straight-forward, and a little bit error-prone, like BMV2 or BMv2 or Bmv2.

If possible, I'd propose gradually converting all existing namespaces to the snake_case style. But I also understand this kind of refactoring might be too annoying to downstream projects that are currently depending on P4C as a library. So I'm insisting on this.

I'll follow whatever decision the larger community comes to, and update this PR accordingly.

[qobilidop]

I don't quite understand the macOS-specific compiling issue. Somehow it's confusing between p4c::std and ::std when std is given. If this is an issue, why does it happen only on macOS?

[asl]

@qobilidop Some things were already places in namespace P4. For example P4::literals. It would make sense to get rid of P4 nested namespace while there.

As for std issue: it looks like there is std nested namespace somewhere inside p4c. As a result when you are inside:

namespace p4c {
  ...
  std::cout << "boo" << std::endl;
  ...
}

std would be referred to p4c::std, not to top-level.

I have not checked the diff, but likely you're having using namespace std or #include <some STL header> inside p4c namespace.

[vlstill]

@qobilidop Some things were already places in namespace P4. For example P4::literals. It would make sense to get rid of P4 nested namespace while there.

I always assumed the P4 namespace is for things bound to the language itself, but I am not sure if that was really the intention. Anyway I agree it would make more sense to get rid of it, then to have P4C::P4.

BTW. Should the namespace be P4C rather than p4c? It seems that a) we use uppercase in namespaces, b) we have agreed to refer to the tool as P4C (except of course for the executable).

[vlstill]

It seems this breaks LOG* printing of P4::IR::ID, P4::Util::SourceInfo and some other values. I'm not sure why that is, but this worked before. I'd guess there is some operator<< that used to be global and for some reason can't be picked by ADT now, probably because it is not in the namespace of the thing it prints, but in P4 namespace. This might not be a bad thing, especially if these operators are supposed to expose the "debug" behavior (which I suspect is the case for IR::ID), but it is still anoying for backend to not have them any more. It might be best to put the operators in the right namespace now and clean those that should not exist in a separate PR. The caveat is I was not able to find the operators.

Then there are values like P4::IR::Direction which also don't have operator<< available. In this case, I'd guess the problem is the operator is generated in the P4 namespace not in P4::IR (where it would presumably be picked by ADL).

A workaround is to use using P4::operator<<.

[asl]

It seems this breaks LOG* printing of P4::IR::ID, P4::Util::SourceInfo and some other values. I'm not sure why that is, but this worked before. I'd guess there is some operator<< that used to be global and for some reason can't be picked by ADT now, probably because it is not in the namespace of the thing it prints, but in P4 namespace.

I think I was already beaten by this before. We define lots of operator<< and expect them to be found via ADL. In particular, boost::format depends on this as it relies on operator<< being available.

See the comment at

p4c/lib/bug_helper.h

Line 93 in 63c4071

// We define operator<< for all T's that have either dbprint() or toString()

[fruffy]

It seems this breaks LOG* printing of P4::IR::ID, P4::Util::SourceInfo and some other values. I'm not sure why that is, but this worked before. I'd guess there is some operator<< that used to be global and for some reason can't be picked by ADT now, probably because it is not in the namespace of the thing it prints, but in P4 namespace.

I think I was already beaten by this before. We define lots of operator<< and expect them to be found via ADL. In particular, boost::format depends on this as it relies on operator<< being available.

See the comment at

p4c/lib/bug_helper.h

Line 93 in 63c4071

// We define operator<< for all T's that have either dbprint() or toString()

I also had issues with this, it's hard to keep track of whats floating around. Any ideas on how to clean it up?

[asl]

I also had issues with this, it's hard to keep track of whats floating around. Any ideas on how to clean it up?

get rid of boost::format :) Though the alternatives are not much better:

• abseil solves this via adding AbslStringify extension point. It could be either in-class method or certainly should be defined in the same namespace so ADL works: https://abseil.io/docs/cpp/guides/abslstringify
• fmtlib allows formatting of user-defined types either via function overload or template specialization: https://fmt.dev/11.0/api/#formatting-user-defined-types Still, certainly both should be defined in the same namespace as the underlying type.

Let me play a bit with possible implementations here and there...

[asl]

The caveat is I was not able to find the operators.

@vlstill They are in lib/stringify.h, enabled globally if there is ->toString() or dbprint method.

[asl]

Some thoughts are here: #4698 (comment)

[qobilidop]

Any remaining issues that must be addressed in this PR? It seems all the identified issues could be addressed in follow-up PRs separately.

[vlstill]

It seems this breaks LOG* printing of P4::IR::ID, P4::Util::SourceInfo and some other values. I'm not sure why that is, but this worked before. I'd guess there is some operator<< that used to be global and for some reason can't be picked by ADT now, probably because it is not in the namespace of the thing it prints, but in P4 namespace.

I think I was already beaten by this before. We define lots of operator<< and expect them to be found via ADL. In particular, boost::format depends on this as it relies on operator<< being available.
See the comment at

p4c/lib/bug_helper.h

Line 93 in 63c4071

// We define operator<< for all T's that have either dbprint() or toString()

I also had issues with this, it's hard to keep track of whats floating around. Any ideas on how to clean it up?

I'd say most of the operators should be defined as close to the class as possible (and definitely in the same namespace). Since there is no good way to pull only some operators (except for namespaces), we should not define an operator<< for a type unless it is generally usable.

For the cases where we really need to pull the operator for a special purpose, I could see defining something like

namespace P4::Dbg { // or P4::Log
... operator<<(..., IR::Node *) { ... }
// all the weird operators that should not work normally
// ...
}

and then we could modify our logging macros to be something like:

#define LOGN(N, X)                                                        \
    [&]{ \
        using P4::Log::operator<<;  \
        if (LOGGING(N)) \
            return ::Log::Detail::fileLogOutput(__FILE__)                  \
                      << ::Log::Detail::OutputLogPrefix(__FILE__, N) << X \
                      << ::Log::Detail::clearPrefix << std::endl          \
        return std::clog; \
    }()

This would need to be tested thoroughly though to make sure the lambda does not break anything (I've use a lambda instead of the do { } while (0) construction as that would make the LOG into a statement, while now it is a expression (which is a bit weird, but changing that could be confusing too).

This way we should get the operators available in logging, but not in normal printouts.

problems fixed

[asl]

we should not define an operator<< for a type unless it is generally usable.

+1

For the cases where we really need to pull the operator for a special purpose, I could see defining something like

My idea was to introduce a special "debug logging stream. There we can overload operator<< for this kind of stream to call dbprint(). This would solve LOG usage. Then we will need to see what's left in tests and IR nodes themselves.

[vlstill]

Applied this to a downstream backend the hard way (not with using namespace P4). One weird problem I had was related to expressions like ASSERT_EQ(...) << exprs where exprs is a vector of things that have dbpring. In this case I had to add using P4::operator<< before the gtest-related includes so that the gtest's stream can see operator<< overload that we have for vector (and which we probably should not really have).

[fruffy]

Great, thanks for the heavy lifting here!