search

microsoft / cpp_client_telemetry

1DS C++ SDK
Apache License 2.0
85 stars 48 forks source link

#define PAL overrides namespace projectNamespace::PAL #1250

Open pablo-msft opened 2 months ago

pablo-msft commented 2 months ago

Problem line: https://github.com/microsoft/cpp_client_telemetry/blame/5068a981e2dc6ebe935e3184c011a1d8edfd6e8a/lib/include/public/ctmacros.hpp#L11

In our project, we have code like:

namespace projectName::PAL {
    class Object {};
}

namespace projectName {
    PAL::Object; // or projectName::PAL::Object - exact same compiler error message
}

When I #include LogManager.hpp, the compiler reports:

error C2039: 'Object': is not a member of 'Microsoft::Applications::Events::PlatformAbstraction'

This telemetry library should try to avoid global namespace conflicts, of which PAL is one, by appending a prefix like MAT_PAL or similar. Additionally, the library should additionally shy away from utilizing macros as namespaces, such as with PAL::getUtcSystemTimeMs() in the library code.

lalitb commented 1 month ago

@pablo-msft Your concerns are valid, unfortunately this is the baggage we have from the legacy implementation, and it can't be changed at this time. As of now, your can undef the macro, or preserve the old PAL macro definition at the place of conflict, and revert afterwards. Let us know if you have further comments ?