Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Support] Rename llvm::support::endianness to llvm::endianness #68174

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

kazutakahirata
Copy link
Contributor

As part of an effort to make our codebase ready for the migration from
llvm::support::endianness to std::endian in C++20, this patch renames
llvm::support::endianness to llvm::endianness.

The intent of this patch is to make fully qualified names less
painful. That is, with this patch, we can just say
llvm::endianness::big rather than llvm::support::endianness::big.

I'm not renaming llvm::support::endianness to llvm::endian because we
have a lot of places with "using namespace support;" where it would be
ambiguous whether "endian" refers to llvm::endian or
llvm::support::endian.

This patch defines several helpers for gradual migration:

namespace llvm {
namespace support {
using endianness = llvm::endianness;
constexpr llvm::endianness big = llvm::endianness::big;
constexpr llvm::endianness little = llvm::endianness::little;
constexpr llvm::endianness native = llvm::endianness::native;

While we are at it, this patch changes the enum to "enum class". The
"enum class" prevents implicit conversions from endianness to bool.
I've fixed three such instances of implicit conversions:

95f4b2a
8de2ecc
a7517e1

As part of an effort to make our codebase ready for the migration from
llvm::support::endianness to std::endian in C++20, this patch renames
llvm::support::endianness to llvm::endianness.

The intent of this patch is to make fully qualified names less
painful.  That is, with this patch, we can just say
llvm::endianness::big rather than llvm::support::endianness::big.

I'm not renaming llvm::support::endianness to llvm::endian because we
have a lot of places with "using namespace support;" where it would be
ambiguous whether "endian" refers to llvm::endian or
llvm::support::endian.

This patch defines several helpers for gradual migration:

  namespace llvm {
  namespace support {
  using endianness = llvm::endianness;
  constexpr llvm::endianness big = llvm::endianness::big;
  constexpr llvm::endianness little = llvm::endianness::little;
  constexpr llvm::endianness native = llvm::endianness::native;

While we are at it, this patch changes the enum to "enum class".  The
"enum class" prevents implicit conversions from endianness to bool.
I've fixed three such instances of implicit conversions:

  95f4b2a
  8de2ecc
  a7517e1
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 4, 2023

@llvm/pr-subscribers-llvm-support

Changes

As part of an effort to make our codebase ready for the migration from
llvm::support::endianness to std::endian in C++20, this patch renames
llvm::support::endianness to llvm::endianness.

The intent of this patch is to make fully qualified names less
painful. That is, with this patch, we can just say
llvm::endianness::big rather than llvm::support::endianness::big.

I'm not renaming llvm::support::endianness to llvm::endian because we
have a lot of places with "using namespace support;" where it would be
ambiguous whether "endian" refers to llvm::endian or
llvm::support::endian.

This patch defines several helpers for gradual migration:

namespace llvm {
namespace support {
using endianness = llvm::endianness;
constexpr llvm::endianness big = llvm::endianness::big;
constexpr llvm::endianness little = llvm::endianness::little;
constexpr llvm::endianness native = llvm::endianness::native;

While we are at it, this patch changes the enum to "enum class". The
"enum class" prevents implicit conversions from endianness to bool.
I've fixed three such instances of implicit conversions:

95f4b2a
8de2ecc
a7517e1


Full diff: https://github.com/llvm/llvm-project/pull/68174.diff

1 Files Affected:

  • (modified) llvm/include/llvm/Support/Endian.h (+10-2)
diff --git a/llvm/include/llvm/Support/Endian.h b/llvm/include/llvm/Support/Endian.h
index 9bfe46022859c9e..e0afeabbcf34a0e 100644
--- a/llvm/include/llvm/Support/Endian.h
+++ b/llvm/include/llvm/Support/Endian.h
@@ -22,14 +22,22 @@
 #include <type_traits>
 
 namespace llvm {
-namespace support {
 
-enum endianness {
+enum class endianness {
   big,
   little,
   native = llvm::sys::IsBigEndianHost ? big : little
 };
 
+namespace support {
+
+// TODO: Remove the following once we are done migrating to llvm::endianness,
+// llvm::endianness::big, etc.
+using endianness = llvm::endianness;
+constexpr llvm::endianness big = llvm::endianness::big;
+constexpr llvm::endianness little = llvm::endianness::little;
+constexpr llvm::endianness native = llvm::endianness::native;
+
 // These are named values for common alignments.
 enum {aligned = 0, unaligned = 1};
 

Copy link
Member

@zero9178 zero9178 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a great incremental strategy to me

@kazutakahirata kazutakahirata merged commit bbdbcd8 into llvm:main Oct 5, 2023
4 checks passed
@kazutakahirata kazutakahirata deleted the pr_endian_enum_class branch October 5, 2023 03:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants