0

[EnumSet] Refactor constructor usages

TL;DR: EnumSet variadic template ctor is being replaced with ctor
accepting std::initializer_list. Some usages are not compatible and this
CL updates them.

Implicit EnumValue->EnumSet conversions and direct calls of variadic
template constructor (e.g. EnumSet(value1,...)) don't work with
initializer_list ctor. This CL replaces them with:
1. [Only variables initializations] EnumSet set = {value1, ...};
2. [Only inside macro invocations] EnumSet({value1, ...})
3. Implicit construction from initializer_list (note that this is a
special case of implicit conversion, intentionally supported in STL
and by clang-tidy warnings):
foo(value1) -> foo({value1});
foo(EnumSet(value1, ...)) -> foo({value1, ...});
4. EnumSet(value1, ...) -> EnumSet{value1, ...};

Note that 3 and 4 are mostly the same, in most places I made a choice
that resembles pre-existing code closer (exceptions are mostly usages of
Sync EnumSets).

Motivation:
EnumSet variadic template constructor is defective in a sense that it
allows code like this to compile:
using ModelTypeSet = EnumSet<ModelType,...>;
void foo(ModelTypeSet set);
foo(ModelType()); // Typo, intended to pass empty ModelTypeSet.

The problem is that constructor is not explicit. Simply making it
explicit will disallow nice STL-like usage:
class C {
  ModelTypeSet set = {HISTORY, BOOKMARKS};
};

STL and other containers under base/containers avoid this problem by
using std::initializer_list, this refactoring does the same for EnumSet.

This CL was uploaded by git cl split.

R=dcheng@chromium.org

Bug: 1444105
Change-Id: I557b77d610a5ef6a4397ae0f811097ed3dfce65a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4523204
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Owners-Override: Daniel Cheng <dcheng@chromium.org>
Auto-Submit: Maksim Moskvitin <mmoskvitin@google.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1143387}
This commit is contained in:
Maksim Moskvitin
2023-05-12 17:57:49 +00:00
committed by Chromium LUCI CQ
parent 271b2cff67
commit b47c7e21ee
3 changed files with 19 additions and 31 deletions

@ -1472,9 +1472,9 @@ TEST(CopyOrMoveOperationDelegateTest, StopRecursionOnCopyError) {
}
TEST(CopyOrMoveOperationDelegateTest, RemoveDestFileOnCopyError) {
FileSystemOperation::CopyOrMoveOptionSet options(
FileSystemOperation::CopyOrMoveOptionSet options = {
storage::FileSystemOperation::CopyOrMoveOption::
kRemovePartiallyCopiedFilesOnError);
kRemovePartiallyCopiedFilesOnError};
CopyOrMoveOperationDelegateTestHelper helper(
"http://foo", kFileSystemTypePersistent, kFileSystemTypePersistent,
options);
@ -1512,9 +1512,9 @@ TEST(CopyOrMoveOperationDelegateTest, RemoveDestFileOnCopyError) {
TEST(CopyOrMoveOperationDelegateTest,
RemoveDestFileOnCrossFilesystemMoveError) {
FileSystemOperation::CopyOrMoveOptionSet options(
FileSystemOperation::CopyOrMoveOptionSet options = {
storage::FileSystemOperation::CopyOrMoveOption::
kRemovePartiallyCopiedFilesOnError);
kRemovePartiallyCopiedFilesOnError};
// Removing destination files on Move errors applies only to cross-filesystem
// moves.
CopyOrMoveOperationDelegateTestHelper helper(

@ -479,8 +479,7 @@ TEST_F(NativeFileUtilTest, PreserveLastModified) {
// Test for copy (nosync).
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::CopyOrMoveFile(
from_file, to_file1,
CopyOrMoveOptionSet(CopyOrMoveOption::kPreserveLastModified),
from_file, to_file1, {CopyOrMoveOption::kPreserveLastModified},
NativeFileUtil::COPY_NOSYNC));
base::File::Info file_info2;
@ -492,8 +491,7 @@ TEST_F(NativeFileUtilTest, PreserveLastModified) {
// Test for copy (sync).
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::CopyOrMoveFile(
from_file, to_file2,
CopyOrMoveOptionSet(CopyOrMoveOption::kPreserveLastModified),
from_file, to_file2, {CopyOrMoveOption::kPreserveLastModified},
NativeFileUtil::COPY_SYNC));
ASSERT_TRUE(FileExists(to_file2));
@ -504,8 +502,7 @@ TEST_F(NativeFileUtilTest, PreserveLastModified) {
// Test for move.
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::CopyOrMoveFile(
from_file, to_file3,
CopyOrMoveOptionSet(CopyOrMoveOption::kPreserveLastModified),
from_file, to_file3, {CopyOrMoveOption::kPreserveLastModified},
NativeFileUtil::MOVE));
ASSERT_TRUE(FileExists(to_file3));
@ -555,8 +552,7 @@ TEST_F(NativeFileUtilTest, PreserveDestinationPermissions) {
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::CopyOrMoveFile(
from_file, to_file,
CopyOrMoveOptionSet(
CopyOrMoveOption::kPreserveDestinationPermissions),
{CopyOrMoveOption::kPreserveDestinationPermissions},
NativeFileUtil::COPY_NOSYNC));
#if BUILDFLAG(IS_POSIX)
ExpectFileHasPermissionsPosix(to_file, old_dest_mode);
@ -568,8 +564,7 @@ TEST_F(NativeFileUtilTest, PreserveDestinationPermissions) {
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::CopyOrMoveFile(
from_file, to_file,
CopyOrMoveOptionSet(
CopyOrMoveOption::kPreserveDestinationPermissions),
{CopyOrMoveOption::kPreserveDestinationPermissions},
NativeFileUtil::COPY_SYNC));
#if BUILDFLAG(IS_POSIX)
ExpectFileHasPermissionsPosix(to_file, old_dest_mode);
@ -581,8 +576,7 @@ TEST_F(NativeFileUtilTest, PreserveDestinationPermissions) {
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::CopyOrMoveFile(
from_file, to_file,
CopyOrMoveOptionSet(
CopyOrMoveOption::kPreserveDestinationPermissions),
{CopyOrMoveOption::kPreserveDestinationPermissions},
NativeFileUtil::MOVE));
#if BUILDFLAG(IS_POSIX)
ExpectFileHasPermissionsPosix(to_file, old_dest_mode);
@ -657,9 +651,8 @@ TEST_F(NativeFileUtilTest, PreserveLastModifiedAndDestinationPermissions) {
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::CopyOrMoveFile(
from_file, to_file1,
CopyOrMoveOptionSet(
CopyOrMoveOption::kPreserveLastModified,
CopyOrMoveOption::kPreserveDestinationPermissions),
{CopyOrMoveOption::kPreserveLastModified,
CopyOrMoveOption::kPreserveDestinationPermissions},
NativeFileUtil::COPY_NOSYNC));
base::File::Info to_file_info;
ASSERT_TRUE(FileExists(to_file1));
@ -677,9 +670,8 @@ TEST_F(NativeFileUtilTest, PreserveLastModifiedAndDestinationPermissions) {
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::CopyOrMoveFile(
from_file, to_file2,
CopyOrMoveOptionSet(
CopyOrMoveOption::kPreserveLastModified,
CopyOrMoveOption::kPreserveDestinationPermissions),
{CopyOrMoveOption::kPreserveLastModified,
CopyOrMoveOption::kPreserveDestinationPermissions},
NativeFileUtil::COPY_SYNC));
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::GetFileInfo(to_file2, &to_file_info));
@ -695,9 +687,8 @@ TEST_F(NativeFileUtilTest, PreserveLastModifiedAndDestinationPermissions) {
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::CopyOrMoveFile(
from_file, to_file3,
CopyOrMoveOptionSet(
CopyOrMoveOption::kPreserveLastModified,
CopyOrMoveOption::kPreserveDestinationPermissions),
{CopyOrMoveOption::kPreserveLastModified,
CopyOrMoveOption::kPreserveDestinationPermissions},
NativeFileUtil::MOVE));
ASSERT_EQ(base::File::FILE_OK,
NativeFileUtil::GetFileInfo(to_file3, &to_file_info));

@ -631,8 +631,7 @@ TEST_F(ObfuscatedFileUtilMemoryDelegateTest, PreserveLastModified_NoSync) {
ASSERT_EQ(base::File::FILE_OK,
file_util()->CopyOrMoveFile(
from_file, to_file,
CopyOrMoveOptionSet(CopyOrMoveOption::kPreserveLastModified),
from_file, to_file, {CopyOrMoveOption::kPreserveLastModified},
nosync));
ASSERT_TRUE(FileExists(to_file));
@ -661,8 +660,7 @@ TEST_F(ObfuscatedFileUtilMemoryDelegateTest, PreserveLastModified_Sync) {
ASSERT_EQ(
base::File::FILE_OK,
file_util()->CopyOrMoveFile(
from_file, to_file,
CopyOrMoveOptionSet(CopyOrMoveOption::kPreserveLastModified), sync));
from_file, to_file, {CopyOrMoveOption::kPreserveLastModified}, sync));
ASSERT_TRUE(FileExists(to_file));
base::File::Info file_info2;
@ -690,8 +688,7 @@ TEST_F(ObfuscatedFileUtilMemoryDelegateTest, PreserveLastModified_Move) {
ASSERT_EQ(
base::File::FILE_OK,
file_util()->CopyOrMoveFile(
from_file, to_file,
CopyOrMoveOptionSet(CopyOrMoveOption::kPreserveLastModified), move));
from_file, to_file, {CopyOrMoveOption::kPreserveLastModified}, move));
ASSERT_TRUE(FileExists(to_file));
base::File::Info file_info2;