[libcxx] Rename PS() macro to avoid clashing with Xtensa register name

[gustavonihei]

Summary

This patch addresses a clash with the PS register from Xtensa
defined in the <specreg.h> header file, which is commonly included in
OS implementation.

Issue identified while building libc++ port for Apache NuttX, targeting
Xtensa-based chips (e.g. Espressif's ESP32).

Signed-off-by: Gustavo Henrique Nihei <[e-mail]>

[gustavonihei]

[libcxx] Add UNICODE definition to be used by <windows.h>

[gustavonihei]

[libcxx] Defined UNICODE to 1 to prevent macro redefinition warning

[mstorsjo]

I think the reasoning/argument here is backwards. I wouldn't want to bring in the TEXT() macro here; what we do here is unrelated to the unicode switch for the Windows APIs.

So the issue is that the PS() macro we use right now can clash with some header on some OS? If that's the case, I don't mind changing it to a slightly longer macro, e.g. PATHSTR() or something like that, but please keep it untangled from the Win32 TEXT()/UNICODE macros.

[gustavonihei]

In D122479#3409111, @mstorsjo wrote:

I think the reasoning/argument here is backwards. I wouldn't want to bring in the TEXT() macro here; what we do here is unrelated to the unicode switch for the Windows APIs.

So the issue is that the PS() macro we use right now can clash with some header on some OS? If that's the case, I don't mind changing it to a slightly longer macro, e.g. PATHSTR() or something like that, but please keep it untangled from the Win32 TEXT()/UNICODE macros.

Yes, it is clashing with another one defined for Xtensa CPUs, commonly included in several OSes:
https://android.googlesource.com/kernel/common/+/fd43fe19b830d6cd0eba08a6c6a5f71a6bd9c1b0/include/asm-xtensa/xtensa/config-linux_be/specreg.h#71
https://github.com/espressif/esp-idf/blob/master/components/xtensa/include/xtensa/specreg.h#L96

I'll update the patch according to your suggestion.
Thanks for the feedback!

[gustavonihei]

Revert previous patch and rename PS() macro according to suggestion from review

[mstorsjo]

Thanks, this looks ok to me. A libc++ approver still will need to give the final verdict though.

[philnik]

Looks OK to me. Please update the title and provide "Your name" <[e-mail]> for attribution if you don't have commit access. Do you want libc++ to officially support some other OS? If not we might break your platform at any time. If you want support you have to provide some CI bots of course.

[gustavonihei]

In D122479#3409225, @philnik wrote:

Looks OK to me. Please update the title and provide "Your name" <[e-mail]> for attribution if you don't have commit access. Do you want libc++ to officially support some other OS? If not we might break your platform at any time. If you want support you have to provide some CI bots of course.

I've update the title and description with information about me for attribution.
Thanks, @philnik!

[ldionne]

LGTM, but I want to mention that this setup is broken for defining a non-namespaced macro with such a short name. It is bound to conflict with tons of other software, and it would be better to fix it in the OS header instead.

libcxx/src/filesystem/filesystem_common.h

 #endif
 #endif
 
 _LIBCPP_DIAGNOSTIC_PUSH
 _LIBCPP_GCC_DIAGNOSTIC_IGNORED("-Wunused-function")
 _LIBCPP_CLANG_DIAGNOSTIC_IGNORED("-Wunused-function")
 
 #if defined(_LIBCPP_WIN32API)
-#define PS(x) (L##x)
-#define PATH_CSTR_FMT "\"%ls\""
+#  define PATHSTR(x) (L##x)
+#  define PATH_CSTR_FMT "\"%ls\""
 #else
-#define PS(x) (x)
-#define PATH_CSTR_FMT "\"%s\""
+#  define PATHSTR(x) (x)
+#  define PATH_CSTR_FMT "\"%s\""
 #endif
 
 _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM
 
 namespace detail {
 
 #if defined(_LIBCPP_WIN32API)
 // Non anonymous, to allow access from two translation units.

libcxx/src/filesystem/operations.cpp

public:
   }
 
   /// \brief Return a view with the "preferred representation" of the current
   ///   element. For example trailing separators are represented as a '.'
   string_view_t operator*() const noexcept {
     switch (State) {
     case PS_BeforeBegin:
     case PS_AtEnd:
-      return PS("");
+      return PATHSTR("");
     case PS_InRootDir:
       if (RawEntry[0] == '\\')
-        return PS("\\");
+        return PATHSTR("\\");
       else
-        return PS("/");
+        return PATHSTR("/");
     case PS_InTrailingSep:
-      return PS("");
+      return PATHSTR("");
     case PS_InRootName:
     case PS_InFilenames:
       return RawEntry;
     }
     __libcpp_unreachable();
   }
 
   explicit operator bool() const noexcept {
#if defined(_LIBCPP_WIN32API)
     if (PosPtr Ret = consumeNetworkRoot(P, End))
       return Ret;
 #endif
     return nullptr;
   }
 };
 
 string_view_pair separate_filename(string_view_t const& s) {
-  if (s == PS(".") || s == PS("..") || s.empty())
-    return string_view_pair{s, PS("")};
+  if (s == PATHSTR(".") || s == PATHSTR("..") || s.empty())
+    return string_view_pair{s, PATHSTR("")};
   auto pos = s.find_last_of('.');
   if (pos == string_view_t::npos || pos == 0)
     return string_view_pair{s, string_view_t{}};
   return string_view_pair{s.substr(0, pos), s.substr(pos)};
 }
 
 string_view_t createView(PosPtr S, PosPtr E) noexcept {
   return {S, static_cast<size_t>(E - S) + 1};

 
 path& path::replace_extension(path const& replacement) {
   path p = extension();
   if (not p.empty()) {
     __pn_.erase(__pn_.size() - p.native().size());
   }
   if (!replacement.empty()) {
     if (replacement.native()[0] != '.') {
-      __pn_ += PS(".");
+      __pn_ += PATHSTR(".");
     }
     __pn_.append(replacement.__pn_);
   }
   return *this;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 // path.decompose
enum PathPartKind : unsigned char {
   PK_Dot,
   PK_DotDot,
   PK_TrailingSep
 };
 
 static PathPartKind ClassifyPathPart(string_view_t Part) {
   if (Part.empty())
     return PK_TrailingSep;
-  if (Part == PS("."))
+  if (Part == PATHSTR("."))
     return PK_Dot;
-  if (Part == PS(".."))
+  if (Part == PATHSTR(".."))
     return PK_DotDot;
-  if (Part == PS("/"))
+  if (Part == PATHSTR("/"))
     return PK_RootSep;
 #if defined(_LIBCPP_WIN32API)
-  if (Part == PS("\\"))
+  if (Part == PATHSTR("\\"))
     return PK_RootSep;
 #endif
   return PK_Filename;
 }
 
 path path::lexically_normal() const {
   if (__pn_.empty())
     return *this;
for (auto PP = PathParser::CreateBegin(__pn_); PP; ++PP) {
     case PK_DotDot: {
       // Only push a ".." element if there are no elements preceding the "..",
       // or if the preceding element is itself "..".
       auto LastKind = LastPartKind();
       if (LastKind == PK_Filename) {
         NewPathSize -= Parts.back().first.size();
         Parts.pop_back();
       } else if (LastKind != PK_RootSep)
-        AddPart(PK_DotDot, PS(".."));
+        AddPart(PK_DotDot, PATHSTR(".."));
       MaybeNeedTrailingSep = LastKind == PK_Filename;
       break;
     }
     case PK_Dot:
     case PK_TrailingSep: {
       MaybeNeedTrailingSep = true;
       break;
     }
     case PK_None:
       __libcpp_unreachable();
     }
   }
   // [fs.path.generic]p6.8: If the path is empty, add a dot.
   if (Parts.empty())
-    return PS(".");
+    return PATHSTR(".");
 
   // [fs.path.generic]p6.7: If the last filename is dot-dot, remove any
   // trailing directory-separator.
   bool NeedTrailingSep = MaybeNeedTrailingSep && LastPartKind() == PK_Filename;
 
   path Result;
   Result.__pn_.reserve(Parts.size() + NewPathSize + NeedTrailingSep);
   for (auto& PK : Parts)
     Result /= PK.first;
 
   if (NeedTrailingSep)
-    Result /= PS("");
+    Result /= PATHSTR("");
 
   Result.make_preferred();
   return Result;
 }
 
 static int DetermineLexicalElementCount(PathParser PP) {
   int Count = 0;
   for (; PP; ++PP) {
     auto Elem = *PP;
-    if (Elem == PS(".."))
+    if (Elem == PATHSTR(".."))
       --Count;
-    else if (Elem != PS(".") && Elem != PS(""))
+    else if (Elem != PATHSTR(".") && Elem != PATHSTR(""))
       ++Count;
   }
   return Count;
 }
 
 path path::lexically_relative(const path& base) const {
   { // perform root-name/root-directory mismatch checks
     auto PP = PathParser::CreateBegin(__pn_);
path path::lexically_relative(const path& base) const {
 
   // Otherwise, determine the number of elements, 'n', which are not dot or
   // dot-dot minus the number of dot-dot elements.
   int ElemCount = DetermineLexicalElementCount(PPBase);
   if (ElemCount < 0)
     return {};
 
   // if n == 0 and (a == end() || a->empty()), returns path("."); otherwise
-  if (ElemCount == 0 && (PP.atEnd() || *PP == PS("")))
-    return PS(".");
+  if (ElemCount == 0 && (PP.atEnd() || *PP == PATHSTR("")))
+    return PATHSTR(".");
 
   // return a path constructed with 'n' dot-dot elements, followed by the the
   // elements of '*this' after the mismatch.
   path Result;
   // FIXME: Reserve enough room in Result that it won't have to re-allocate.
   while (ElemCount--)
-    Result /= PS("..");
+    Result /= PATHSTR("..");
   for (; PP; ++PP)
     Result /= *PP;
   return Result;
 }
 
 ////////////////////////////////////////////////////////////////////////////
 // path.comparisons
 static int CompareRootName(PathParser *LHS, PathParser *RHS) {
   if (!LHS->inRootName() && !RHS->inRootName())
     return 0;
 
   auto GetRootName = [](PathParser *Parser) -> string_view_t {
-    return Parser->inRootName() ? **Parser : PS("");
+    return Parser->inRootName() ? **Parser : PATHSTR("");
   };
   int res = GetRootName(LHS).compare(GetRootName(RHS));
   ConsumeRootName(LHS);
   ConsumeRootName(RHS);
   return res;
 }
 
 static int CompareRootDir(PathParser *LHS, PathParser *RHS) {