Rename factory functions from 'Create' to 'Make' (#3706)

Similar to #3705, we actually have a mix of `Make` and `Create` in
factory functions too, so this PR is normalizing on `Make`. It's
intended to be consistent with the naming choice for Carbon factory
functions.

Note, MakeSyntheticBlock is the only one I feel a little weird about
because llvm's own APIs use Create, and this is essentially wrapping
LLVM calls. But the flipside is it also feels like a vague line to draw,
when we also differ from LLVM coding style in other ways.
This commit is contained in:
Jon Ross-Perkins
2024-02-14 18:26:56 +00:00
committed by GitHub
parent 4873160acc
commit 1974e44fd9
24 changed files with 66 additions and 68 deletions
+6 -6
View File
@@ -42,7 +42,7 @@ namespace Carbon::Internal {
//
// // OPTIONAL: To expose the ability to create an instance from the raw
// // enumerator (for unusual use cases), add this:
// using EnumBase::Create;
// using EnumBase::Make;
//
// // Plus, anything else you wish to include.
// };
@@ -141,12 +141,12 @@ class EnumBase : public Printable<DerivedT> {
protected:
// The default constructor is explicitly defaulted (and constexpr) as a
// protected constructor to allow derived classes to be constructed but not
// the base itself. This should only be used in the `Create` function below.
// the base itself. This should only be used in the `Make` function below.
constexpr EnumBase() = default;
// Create an instance from the raw enumerator. Mainly used internally, but may
// be exposed for unusual use cases.
static constexpr auto Create(RawEnumType value) -> EnumType {
static constexpr auto Make(RawEnumType value) -> EnumType {
EnumType result;
result.value_ = value;
return result;
@@ -161,7 +161,7 @@ class EnumBase : public Printable<DerivedT> {
// Convert from the underlying integer type. Derived types can choose to
// expose this as part of their API.
static constexpr auto FromInt(UnderlyingType value) -> EnumType {
return Create(static_cast<RawEnumType>(value));
return Make(static_cast<RawEnumType>(value));
}
private:
@@ -211,7 +211,7 @@ class EnumBase : public Printable<DerivedT> {
// constant.
#define CARBON_ENUM_CONSTANT_DEFINITION(EnumClassName, Name) \
constexpr EnumClassName EnumClassName::Name = \
EnumClassName::Create(RawEnumType::Name);
EnumClassName::Make(RawEnumType::Name);
// Alternatively, use this within the Carbon enum class body to declare and
// define each named constant. Due to type completeness constraints, this will
@@ -221,7 +221,7 @@ class EnumBase : public Printable<DerivedT> {
// `EnumBase` base class.
#define CARBON_INLINE_ENUM_CONSTANT_DEFINITION(Name) \
static constexpr const typename Base::EnumType& Name = \
Base::Create(Base::RawEnumType::Name);
Base::Make(Base::RawEnumType::Name);
// Use this in the `.cpp` file for an enum class to start the definition of the
// constant names array for each enumerator. It is followed by the desired
+3 -3
View File
@@ -17,7 +17,7 @@ class IndirectValue;
// Creates and returns an IndirectValue that holds the value returned by
// `callable()`.
template <typename Callable>
auto CreateIndirectValue(Callable callable)
auto MakeIndirectValue(Callable callable)
-> IndirectValue<std::decay_t<decltype(callable())>>;
// An IndirectValue<T> object stores a T value, using a layer of indirection
@@ -88,7 +88,7 @@ class IndirectValue {
static_assert(std::is_object_v<T>, "T must be an object type");
template <typename Callable>
friend auto CreateIndirectValue(Callable callable)
friend auto MakeIndirectValue(Callable callable)
-> IndirectValue<std::decay_t<decltype(callable())>>;
template <typename... Args>
@@ -98,7 +98,7 @@ class IndirectValue {
};
template <typename Callable>
auto CreateIndirectValue(Callable callable)
auto MakeIndirectValue(Callable callable)
-> IndirectValue<std::decay_t<decltype(callable())>> {
using T = std::decay_t<decltype(callable())>;
return IndirectValue<T>(std::unique_ptr<T>(new T(callable())));
+2 -2
View File
@@ -35,7 +35,7 @@ struct NonMovable {
TEST(IndirectValueTest, Create) {
IndirectValue<NonMovable> v =
CreateIndirectValue([] { return NonMovable(42); });
MakeIndirectValue([] { return NonMovable(42); });
EXPECT_EQ(v->i, 42);
}
@@ -45,7 +45,7 @@ auto GetIntReference() -> const int& {
}
TEST(IndirectValueTest, CreateWithDecay) {
auto v = CreateIndirectValue(GetIntReference);
auto v = MakeIndirectValue(GetIntReference);
EXPECT_TRUE((std::is_same_v<decltype(v), IndirectValue<int>>));
EXPECT_EQ(*v, 42);
}
+1 -1
View File
@@ -99,7 +99,7 @@ void LanguageServer::OnDocumentSymbol(
vfs.addFile(file, /*mtime=*/0,
llvm::MemoryBuffer::getMemBufferCopy(files_.at(file)));
auto buf = SourceBuffer::CreateFromFile(vfs, file, NullDiagnosticConsumer());
auto buf = SourceBuffer::MakeFromFile(vfs, file, NullDiagnosticConsumer());
auto lexed = Lex::Lex(value_stores, *buf, NullDiagnosticConsumer());
auto parsed = Parse::Parse(lexed, NullDiagnosticConsumer(), nullptr);
std::vector<clang::clangd::DocumentSymbol> result;
+2 -3
View File
@@ -13,9 +13,8 @@
namespace Carbon {
auto CodeGen::Create(llvm::Module& module, llvm::StringRef target_triple,
llvm::raw_pwrite_stream& errors)
-> std::optional<CodeGen> {
auto CodeGen::Make(llvm::Module& module, llvm::StringRef target_triple,
llvm::raw_pwrite_stream& errors) -> std::optional<CodeGen> {
std::string error;
const llvm::Target* target =
llvm::TargetRegistry::lookupTarget(target_triple, error);
+2 -2
View File
@@ -12,8 +12,8 @@ namespace Carbon {
class CodeGen {
public:
static auto Create(llvm::Module& module, llvm::StringRef target_triple,
llvm::raw_pwrite_stream& errors) -> std::optional<CodeGen>;
static auto Make(llvm::Module& module, llvm::StringRef target_triple,
llvm::raw_pwrite_stream& errors) -> std::optional<CodeGen>;
// Generates the object code file.
// Returns false in case of failure, and any information about the failure is
+5 -5
View File
@@ -410,12 +410,12 @@ class Driver::CompilationUnit {
// Loads source and lexes it. Returns true on success.
auto RunLex() -> bool {
LogCall("SourceBuffer::CreateFromFile", [&] {
LogCall("SourceBuffer::MakeFromFile", [&] {
if (input_file_name_ == "-") {
source_ = SourceBuffer::CreateFromStdin(*consumer_);
source_ = SourceBuffer::MakeFromStdin(*consumer_);
} else {
source_ = SourceBuffer::CreateFromFile(driver_->fs_, input_file_name_,
*consumer_);
source_ = SourceBuffer::MakeFromFile(driver_->fs_, input_file_name_,
*consumer_);
}
});
if (!source_) {
@@ -514,7 +514,7 @@ class Driver::CompilationUnit {
CARBON_VLOG() << "*** CodeGen ***\n";
std::optional<CodeGen> codegen =
CodeGen::Create(*module_, options_.target, driver_->error_stream_);
CodeGen::Make(*module_, options_.target, driver_->error_stream_);
if (!codegen) {
return false;
}
+7 -7
View File
@@ -46,8 +46,8 @@ class DriverTest : public testing::Test {
test_tmpdir_ = tmpdir_env;
}
auto CreateTestFile(llvm::StringRef text,
llvm::StringRef file_name = "test_file.carbon")
auto MakeTestFile(llvm::StringRef text,
llvm::StringRef file_name = "test_file.carbon")
-> llvm::StringRef {
fs_.addFile(file_name, /*ModificationTime=*/0,
llvm::MemoryBuffer::getMemBuffer(text));
@@ -117,7 +117,7 @@ TEST_F(DriverTest, CompileCommandErrors) {
// Invalid output filename. No reliably error message here.
// TODO: Likely want a different filename on Windows.
auto empty_file = CreateTestFile("");
auto empty_file = MakeTestFile("");
EXPECT_FALSE(
driver_.RunCommand({"compile", "--output=/dev/empty", empty_file}));
EXPECT_THAT(test_error_stream_.TakeStr(),
@@ -125,7 +125,7 @@ TEST_F(DriverTest, CompileCommandErrors) {
}
TEST_F(DriverTest, DumpTokens) {
auto file = CreateTestFile("Hello World");
auto file = MakeTestFile("Hello World");
EXPECT_TRUE(
driver_.RunCommand({"compile", "--phase=lex", "--dump-tokens", file}));
EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
@@ -135,7 +135,7 @@ TEST_F(DriverTest, DumpTokens) {
}
TEST_F(DriverTest, DumpParseTree) {
auto file = CreateTestFile("var v: i32 = 42;");
auto file = MakeTestFile("var v: i32 = 42;");
EXPECT_TRUE(driver_.RunCommand(
{"compile", "--phase=parse", "--dump-parse-tree", file}));
EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
@@ -146,7 +146,7 @@ TEST_F(DriverTest, DumpParseTree) {
TEST_F(DriverTest, StdoutOutput) {
// Use explicit filenames so we can look for those to validate output.
CreateTestFile("fn Main() -> i32 { return 0; }", "test.carbon");
MakeTestFile("fn Main() -> i32 { return 0; }", "test.carbon");
EXPECT_TRUE(driver_.RunCommand({"compile", "--output=-", "test.carbon"}));
EXPECT_THAT(test_error_stream_.TakeStr(), StrEq(""));
@@ -170,7 +170,7 @@ TEST_F(DriverTest, FileOutput) {
// Use explicit filenames as the default output filename is computed from
// this, and we can use this to validate output.
CreateTestFile("fn Main() -> i32 { return 0; }", "test.carbon");
MakeTestFile("fn Main() -> i32 { return 0; }", "test.carbon");
// Object output (the default) uses `.o`.
// TODO: This should actually reflect the platform defaults.
+3 -3
View File
@@ -87,7 +87,7 @@ class [[clang::internal_linkage]] Lexer {
// Explicitly kept out-of-line because this is a significant loop that is
// useful to have in the profile and it doesn't simplify by inlining at all.
// But because it can, the compiler will flatten this otherwise.
[[gnu::noinline]] auto CreateLines(llvm::StringRef source_text) -> void;
[[gnu::noinline]] auto MakeLines(llvm::StringRef source_text) -> void;
auto current_line() -> LineIndex { return LineIndex(line_index_); }
@@ -647,7 +647,7 @@ auto Lexer::Lex() && -> TokenizedBuffer {
llvm::StringRef source_text = buffer_.source_->text();
// First build up our line data structures.
CreateLines(source_text);
MakeLines(source_text);
ssize_t position = 0;
LexFileStart(source_text, position);
@@ -663,7 +663,7 @@ auto Lexer::Lex() && -> TokenizedBuffer {
return std::move(buffer_);
}
auto Lexer::CreateLines(llvm::StringRef source_text) -> void {
auto Lexer::MakeLines(llvm::StringRef source_text) -> void {
// We currently use `memchr` here which typically is well optimized to use
// SIMD or other significantly faster than byte-wise scanning. We also use
// carefully selected variables and the `ssize_t` type for performance and
+2 -2
View File
@@ -396,8 +396,8 @@ class LexerBenchHelper {
auto MakeSourceBuffer(llvm::StringRef text) -> SourceBuffer {
CARBON_CHECK(fs_.addFile(filename_, /*ModificationTime=*/0,
llvm::MemoryBuffer::getMemBuffer(text)));
return std::move(*SourceBuffer::CreateFromFile(
fs_, filename_, ConsoleDiagnosticConsumer()));
return std::move(*SourceBuffer::MakeFromFile(fs_, filename_,
ConsoleDiagnosticConsumer()));
}
SharedValueStores value_stores_;
+1 -1
View File
@@ -32,7 +32,7 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
llvm::MemoryBuffer::getMemBuffer(data_ref, /*BufferName=*/TestFileName,
/*RequiresNullTerminator=*/false)));
auto source =
SourceBuffer::CreateFromFile(fs, TestFileName, NullDiagnosticConsumer());
SourceBuffer::MakeFromFile(fs, TestFileName, NullDiagnosticConsumer());
SharedValueStores value_stores;
auto buffer = Lex::Lex(value_stores, *source, NullDiagnosticConsumer());
+1 -1
View File
@@ -39,7 +39,7 @@ class LexerTest : public ::testing::Test {
std::string filename = llvm::formatv("test{0}.carbon", ++file_index_);
CARBON_CHECK(fs_.addFile(filename, /*ModificationTime=*/0,
llvm::MemoryBuffer::getMemBuffer(text)));
source_storage_.push_front(std::move(*SourceBuffer::CreateFromFile(
source_storage_.push_front(std::move(*SourceBuffer::MakeFromFile(
fs_, filename, ConsoleDiagnosticConsumer())));
return source_storage_.front();
}
+1 -1
View File
@@ -71,7 +71,7 @@ auto FunctionContext::GetBlockArg(SemIR::InstBlockId block_id,
return phi;
}
auto FunctionContext::CreateSyntheticBlock() -> llvm::BasicBlock* {
auto FunctionContext::MakeSyntheticBlock() -> llvm::BasicBlock* {
synthetic_block_ = llvm::BasicBlock::Create(llvm_context(), "", function_);
return synthetic_block_;
}
+1 -1
View File
@@ -78,7 +78,7 @@ class FunctionContext {
// a block should only ever have a single predecessor, and is used when we
// need multiple `llvm::BasicBlock`s to model the linear control flow in a
// single SemIR::File block.
auto CreateSyntheticBlock() -> llvm::BasicBlock*;
auto MakeSyntheticBlock() -> llvm::BasicBlock*;
// Determine whether block is the most recently created synthetic block.
auto IsCurrentSyntheticBlock(llvm::BasicBlock* block) -> bool {
+1 -1
View File
@@ -107,7 +107,7 @@ auto HandleBranchIf(FunctionContext& context, SemIR::InstId /*inst_id*/,
SemIR::BranchIf inst) -> void {
llvm::Value* cond = context.GetValue(inst.cond_id);
llvm::BasicBlock* then_block = context.GetBlock(inst.target_id);
llvm::BasicBlock* else_block = context.CreateSyntheticBlock();
llvm::BasicBlock* else_block = context.MakeSyntheticBlock();
context.builder().CreateCondBr(cond, then_block, else_block);
context.builder().SetInsertPoint(else_block);
}
+1 -1
View File
@@ -75,7 +75,7 @@ class NodeKind : public CARBON_ENUM_BASE(NodeKind) {
static const int ValidCount;
using EnumBase::AsInt;
using EnumBase::Create;
using EnumBase::Make;
class Definition;
+1 -1
View File
@@ -29,7 +29,7 @@ extern "C" int LLVMFuzzerTestOneInput(const unsigned char* data,
llvm::MemoryBuffer::getMemBuffer(data_ref, /*BufferName=*/TestFileName,
/*RequiresNullTerminator=*/false)));
auto source =
SourceBuffer::CreateFromFile(fs, TestFileName, NullDiagnosticConsumer());
SourceBuffer::MakeFromFile(fs, TestFileName, NullDiagnosticConsumer());
// Lex the input.
SharedValueStores value_stores;
+2 -2
View File
@@ -32,8 +32,8 @@ class TreeTest : public ::testing::Test {
auto GetSourceBuffer(llvm::StringRef t) -> SourceBuffer& {
CARBON_CHECK(fs_.addFile("test.carbon", /*ModificationTime=*/0,
llvm::MemoryBuffer::getMemBuffer(t)));
source_storage_.push_front(std::move(
*SourceBuffer::CreateFromFile(fs_, "test.carbon", consumer_)));
source_storage_.push_front(
std::move(*SourceBuffer::MakeFromFile(fs_, "test.carbon", consumer_)));
return source_storage_.front();
}
+2 -2
View File
@@ -27,8 +27,8 @@ class TypedNodeTest : public ::testing::Test {
auto GetSourceBuffer(llvm::StringRef t) -> SourceBuffer& {
CARBON_CHECK(fs_.addFile("test.carbon", /*ModificationTime=*/0,
llvm::MemoryBuffer::getMemBuffer(t)));
source_storage_.push_front(std::move(
*SourceBuffer::CreateFromFile(fs_, "test.carbon", consumer_)));
source_storage_.push_front(
std::move(*SourceBuffer::MakeFromFile(fs_, "test.carbon", consumer_)));
return source_storage_.front();
}
+1 -1
View File
@@ -127,7 +127,7 @@ class Inst : public Printable<Inst> {
// NOLINTNEXTLINE(google-explicit-constructor)
Inst(TypedInst typed_inst)
// kind_ is always overwritten below.
: kind_(InstKind::Create({})),
: kind_(InstKind::Make({})),
type_id_(TypeId::Invalid),
arg0_(InstId::InvalidIndex),
arg1_(InstId::InvalidIndex) {
+1 -1
View File
@@ -60,7 +60,7 @@ class InstKind : public CARBON_ENUM_BASE(InstKind) {
-> Definition<TypedNodeId>;
using EnumBase::AsInt;
using EnumBase::Create;
using EnumBase::Make;
// Returns the name to use for this instruction kind in Semantics IR.
auto ir_name() const -> llvm::StringLiteral;
+8 -8
View File
@@ -18,15 +18,15 @@ struct FilenameTranslator : DiagnosticLocationTranslator<llvm::StringRef> {
};
} // namespace
auto SourceBuffer::CreateFromStdin(DiagnosticConsumer& consumer)
auto SourceBuffer::MakeFromStdin(DiagnosticConsumer& consumer)
-> std::optional<SourceBuffer> {
return CreateFromMemoryBuffer(llvm::MemoryBuffer::getSTDIN(), "<stdin>",
/*is_regular_file=*/false, consumer);
return MakeFromMemoryBuffer(llvm::MemoryBuffer::getSTDIN(), "<stdin>",
/*is_regular_file=*/false, consumer);
}
auto SourceBuffer::CreateFromFile(llvm::vfs::FileSystem& fs,
llvm::StringRef filename,
DiagnosticConsumer& consumer)
auto SourceBuffer::MakeFromFile(llvm::vfs::FileSystem& fs,
llvm::StringRef filename,
DiagnosticConsumer& consumer)
-> std::optional<SourceBuffer> {
FilenameTranslator translator;
DiagnosticEmitter<llvm::StringRef> emitter(translator, consumer);
@@ -54,12 +54,12 @@ auto SourceBuffer::CreateFromFile(llvm::vfs::FileSystem& fs,
bool is_regular_file = status->isRegularFile();
int64_t size = is_regular_file ? status->getSize() : -1;
return CreateFromMemoryBuffer(
return MakeFromMemoryBuffer(
(*file)->getBuffer(filename, size, /*RequiresNullTerminator=*/false),
filename, is_regular_file, consumer);
}
auto SourceBuffer::CreateFromMemoryBuffer(
auto SourceBuffer::MakeFromMemoryBuffer(
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> buffer,
llvm::StringRef filename, bool is_regular_file,
DiagnosticConsumer& consumer) -> std::optional<SourceBuffer> {
+4 -5
View File
@@ -36,14 +36,13 @@ class SourceBuffer {
public:
// Opens and reads the contents of stdin. Returns a SourceBuffer on success.
// Prints an error and returns nullopt on failure.
static auto CreateFromStdin(DiagnosticConsumer& consumer)
static auto MakeFromStdin(DiagnosticConsumer& consumer)
-> std::optional<SourceBuffer>;
// Opens the requested file. Returns a SourceBuffer on success. Prints an
// error and returns nullopt on failure.
static auto CreateFromFile(llvm::vfs::FileSystem& fs,
llvm::StringRef filename,
DiagnosticConsumer& consumer)
static auto MakeFromFile(llvm::vfs::FileSystem& fs, llvm::StringRef filename,
DiagnosticConsumer& consumer)
-> std::optional<SourceBuffer>;
// Use one of the factory functions above to create a source buffer.
@@ -60,7 +59,7 @@ class SourceBuffer {
private:
// Creates a `SourceBuffer` from the given `llvm::MemoryBuffer`. Prints an
// error and returns nullopt on failure.
static auto CreateFromMemoryBuffer(
static auto MakeFromMemoryBuffer(
llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> buffer,
llvm::StringRef filename, bool is_regular_file,
DiagnosticConsumer& consumer) -> std::optional<SourceBuffer>;
+8 -8
View File
@@ -17,8 +17,8 @@ static constexpr llvm::StringLiteral TestFileName = "test.carbon";
TEST(SourceBufferTest, MissingFile) {
llvm::vfs::InMemoryFileSystem fs;
auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName,
ConsoleDiagnosticConsumer());
auto buffer =
SourceBuffer::MakeFromFile(fs, TestFileName, ConsoleDiagnosticConsumer());
EXPECT_FALSE(buffer);
}
@@ -27,8 +27,8 @@ TEST(SourceBufferTest, SimpleFile) {
CARBON_CHECK(fs.addFile(TestFileName, /*ModificationTime=*/0,
llvm::MemoryBuffer::getMemBuffer("Hello World")));
auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName,
ConsoleDiagnosticConsumer());
auto buffer =
SourceBuffer::MakeFromFile(fs, TestFileName, ConsoleDiagnosticConsumer());
ASSERT_TRUE(buffer);
EXPECT_EQ(TestFileName, buffer->filename());
@@ -44,8 +44,8 @@ TEST(SourceBufferTest, NoNull) {
/*BufferName=*/"",
/*RequiresNullTerminator=*/false)));
auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName,
ConsoleDiagnosticConsumer());
auto buffer =
SourceBuffer::MakeFromFile(fs, TestFileName, ConsoleDiagnosticConsumer());
ASSERT_TRUE(buffer);
EXPECT_EQ(TestFileName, buffer->filename());
@@ -57,8 +57,8 @@ TEST(SourceBufferTest, EmptyFile) {
CARBON_CHECK(fs.addFile(TestFileName, /*ModificationTime=*/0,
llvm::MemoryBuffer::getMemBuffer("")));
auto buffer = SourceBuffer::CreateFromFile(fs, TestFileName,
ConsoleDiagnosticConsumer());
auto buffer =
SourceBuffer::MakeFromFile(fs, TestFileName, ConsoleDiagnosticConsumer());
ASSERT_TRUE(buffer);
EXPECT_EQ(TestFileName, buffer->filename());