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

Update flags enums within tables to follow precedence/guideline by adding Flags suffix keeping specific operators (&, &=, |, |=) #6153

Open
ahsonkhan opened this issue Oct 30, 2024 · 0 comments
Assignees
Labels

Comments

@ahsonkhan
Copy link
Member

ahsonkhan commented Oct 30, 2024

Use this as an example, from Storage:

enum class ListBlobContainersIncludeFlags
{
None = 0,
Metadata = 1,
Deleted = 2,
System = 4,
};
inline ListBlobContainersIncludeFlags operator|(
ListBlobContainersIncludeFlags lhs,
ListBlobContainersIncludeFlags rhs)
{
using type = std::underlying_type_t<ListBlobContainersIncludeFlags>;
return static_cast<ListBlobContainersIncludeFlags>(
static_cast<type>(lhs) | static_cast<type>(rhs));
}
inline ListBlobContainersIncludeFlags& operator|=(
ListBlobContainersIncludeFlags& lhs,
ListBlobContainersIncludeFlags rhs)
{
lhs = lhs | rhs;
return lhs;
}
inline ListBlobContainersIncludeFlags operator&(
ListBlobContainersIncludeFlags lhs,
ListBlobContainersIncludeFlags rhs)
{
using type = std::underlying_type_t<ListBlobContainersIncludeFlags>;
return static_cast<ListBlobContainersIncludeFlags>(
static_cast<type>(lhs) & static_cast<type>(rhs));
}
inline ListBlobContainersIncludeFlags& operator&=(
ListBlobContainersIncludeFlags& lhs,
ListBlobContainersIncludeFlags rhs)
{
lhs = lhs & rhs;
return lhs;
}

  1. Flags enums should be suffixed with Flags:
    Change Request: Bitflag enums must be named with Flags suffix for C++ azure-sdk#2315

  2. If we are keeping these flags enum, I believe we should keep the operators specific to the two enums, duplicating them, rather than "templatizing" them. That's what Storage did for their Flags enums, so there's precedence.

And rather than having all the extra bitwise operators (like XOR or NOT), we can simplify to only the 4 that's most likely to be needed by customers (OR/AND): &, &=, |, |=

Here are the set of enums that need to be updated:

  • enum class AccountSasPermissions
  • enum class AccountSasServices
  • enum class AccountSasResourceType
  • enum class TablesSasPermissions

This one is correct (although it doesn't seem to be used anywhere within QueryTables() - #6155):

  • enum class QueryTablesIncludeFlags

/**
* @brief The list of permissions that can be set for an account's access policy.
*/
enum class AccountSasPermissions
{
/**
* @brief Indicates that Read is permitted.
*/
Read = 1,
/**
* @brief Indicates that Write is permitted.
*/
Write = 2,
/**
* @brief Indicates that Delete is permitted.
*/
Delete = 4,
/**
* @brief Indicates that Add is permitted.
*/
Add = 8,
/**
* @brief Indicates that List is permitted.
*/
List = 16,
/**
* @brief Indicates that Update is permitted.
*/
Update = 32,
/**
* @brief Indicates that all permissions are set.
*/
All = ~0,
};

cc @gearama, @LarryOsterman, @RickWinter

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants