Skip to content

Commit

Permalink
Switch to trailing returns in toolchain and related code. (carbon-lan…
Browse files Browse the repository at this point in the history
…guage#4919)

Also makes the style guide explicitly comment on void, but this was the
intent IIRC because it matches Carbon's `-> ()` (and "always" versus
"except for void", which we definitely went back and forth on).

Includes adjusting function pointers, which I definitely forget this
syntax works sometimes.

Excludes utils/tree_sitter/src/scanner.c because it claims to be C, but
really we should probably fix that to be cpp.
  • Loading branch information
jonmeow authored Feb 11, 2025
1 parent 316a6c5 commit 2fef1cb
Show file tree
Hide file tree
Showing 38 changed files with 291 additions and 253 deletions.
207 changes: 114 additions & 93 deletions common/command_line.cpp

Large diffs are not rendered by default.

116 changes: 63 additions & 53 deletions common/command_line.h
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ auto operator<<(llvm::raw_ostream& output, ParseResult result)

// Actions are stored in data structures so we use an owning closure to model
// them.
using ActionT = std::function<void()>;
using ActionT = std::function<auto()->void>;

// The core argument info used to render help and other descriptive
// information. This is used for both options and positional arguments.
Expand Down Expand Up @@ -294,17 +294,17 @@ class ArgBuilder {
public:
// When marked as required, if an argument is not provided explicitly in the
// command line the parse will produce an error.
void Required(bool is_required);
auto Required(bool is_required) -> void;

// An argument can be hidden from the help output.
void HelpHidden(bool is_help_hidden);
auto HelpHidden(bool is_help_hidden) -> void;

// Sets a meta-action to run when this argument is parsed. This is used to
// set up arguments like `--help` or `--version` that can be entirely
// handled during parsing and rather than produce parsed information about
// the command line, override that for some custom behavior.
template <typename T>
void MetaAction(T action);
auto MetaAction(T action) -> void;

protected:
friend class CommandBuilder;
Expand All @@ -323,12 +323,12 @@ class FlagBuilder : public ArgBuilder {
// Flags can be defaulted to true. However, flags always have *some*
// default, this merely customizes which value is default. If uncustomized,
// the default of a flag is false.
void Default(bool flag_value);
auto Default(bool flag_value) -> void;

// Configures the argument to store a parsed value in the provided storage.
//
// This must be called on the builder.
void Set(bool* flag);
auto Set(bool* flag) -> void;

private:
using ArgBuilder::ArgBuilder;
Expand All @@ -345,23 +345,23 @@ class IntegerArgBuilder : public ArgBuilder {
// below, this value will be used whenever the argument occurs without an
// explicit value, but unless the argument is parsed nothing will be
// appended.
void Default(int integer_value);
auto Default(int integer_value) -> void;

// Configures the argument to store a parsed value in the provided storage.
// Each time the argument is parsed, it will write a new value to this
// storage.
//
// Exactly one of this method or `Append` below must be configured for the
// argument.
void Set(int* integer);
auto Set(int* integer) -> void;

// Configures the argument to append a parsed value to the provided
// container. Each time the argument is parsed, a new value will be
// appended.
//
// Exactly one of this method or `Set` above must be configured for the
// argument.
void Append(llvm::SmallVectorImpl<int>* sequence);
auto Append(llvm::SmallVectorImpl<int>* sequence) -> void;

private:
using ArgBuilder::ArgBuilder;
Expand All @@ -378,23 +378,23 @@ class StringArgBuilder : public ArgBuilder {
// below, this value will be used whenever the argument occurs without an
// explicit value, but unless the argument is parsed nothing will be
// appended.
void Default(llvm::StringRef string_value);
auto Default(llvm::StringRef string_value) -> void;

// Configures the argument to store a parsed value in the provided storage.
// Each time the argument is parsed, it will write a new value to this
// storage.
//
// Exactly one of this method or `Append` below must be configured for the
// argument.
void Set(llvm::StringRef* string);
auto Set(llvm::StringRef* string) -> void;

// Configures the argument to append a parsed value to the provided
// container. Each time the argument is parsed, a new value will be
// appended.
//
// Exactly one of this method or `Set` above must be configured for the
// argument.
void Append(llvm::SmallVectorImpl<llvm::StringRef>* sequence);
auto Append(llvm::SmallVectorImpl<llvm::StringRef>* sequence) -> void;

private:
using ArgBuilder::ArgBuilder;
Expand Down Expand Up @@ -463,7 +463,7 @@ class OneOfArgBuilder : public ArgBuilder {
// optional, and also will cause that value to be stored into the result
// even if the argument is not parsed explicitly.
template <typename T, typename U, size_t N>
void SetOneOf(const OneOfValueT<U> (&values)[N], T* result);
auto SetOneOf(const OneOfValueT<U> (&values)[N], T* result) -> void;

// Configures the argument to append a parsed value to the provided
// container. Each time the argument is parsed, a new value will be
Expand Down Expand Up @@ -491,15 +491,15 @@ class OneOfArgBuilder : public ArgBuilder {
// However, appending one-of arguments cannot use a default. The values must
// always be explicitly parsed.
template <typename T, typename U, size_t N>
void AppendOneOf(const OneOfValueT<U> (&values)[N],
llvm::SmallVectorImpl<T>* sequence);
auto AppendOneOf(const OneOfValueT<U> (&values)[N],
llvm::SmallVectorImpl<T>* sequence) -> void;

private:
using ArgBuilder::ArgBuilder;

template <typename U, size_t N, typename MatchT, size_t... Indices>
void OneOfImpl(const OneOfValueT<U> (&input_values)[N], MatchT match,
std::index_sequence<Indices...> /*indices*/);
auto OneOfImpl(const OneOfValueT<U> (&input_values)[N], MatchT match,
std::index_sequence<Indices...> /*indices*/) -> void;
};

// The extended info for a command, including for a subcommand.
Expand Down Expand Up @@ -593,37 +593,45 @@ class CommandBuilder {
public:
using Kind = CommandKind;

void AddFlag(const ArgInfo& info,
llvm::function_ref<void(FlagBuilder&)> build);
void AddIntegerOption(const ArgInfo& info,
llvm::function_ref<void(IntegerArgBuilder&)> build);
void AddStringOption(const ArgInfo& info,
llvm::function_ref<void(StringArgBuilder&)> build);
void AddOneOfOption(const ArgInfo& info,
llvm::function_ref<void(OneOfArgBuilder&)> build);
void AddMetaActionOption(const ArgInfo& info,
llvm::function_ref<void(ArgBuilder&)> build);

void AddIntegerPositionalArg(
const ArgInfo& info, llvm::function_ref<void(IntegerArgBuilder&)> build);
void AddStringPositionalArg(
const ArgInfo& info, llvm::function_ref<void(StringArgBuilder&)> build);
void AddOneOfPositionalArg(const ArgInfo& info,
llvm::function_ref<void(OneOfArgBuilder&)> build);

void AddSubcommand(const CommandInfo& info,
llvm::function_ref<void(CommandBuilder&)> build);
auto AddFlag(const ArgInfo& info,
llvm::function_ref<auto(FlagBuilder&)->void> build) -> void;
auto AddIntegerOption(
const ArgInfo& info,
llvm::function_ref<auto(IntegerArgBuilder&)->void> build) -> void;
auto AddStringOption(const ArgInfo& info,
llvm::function_ref<auto(StringArgBuilder&)->void> build)
-> void;
auto AddOneOfOption(const ArgInfo& info,
llvm::function_ref<auto(OneOfArgBuilder&)->void> build)
-> void;
auto AddMetaActionOption(const ArgInfo& info,
llvm::function_ref<auto(ArgBuilder&)->void> build)
-> void;

auto AddIntegerPositionalArg(
const ArgInfo& info,
llvm::function_ref<auto(IntegerArgBuilder&)->void> build) -> void;
auto AddStringPositionalArg(
const ArgInfo& info,
llvm::function_ref<auto(StringArgBuilder&)->void> build) -> void;
auto AddOneOfPositionalArg(
const ArgInfo& info,
llvm::function_ref<auto(OneOfArgBuilder&)->void> build) -> void;

auto AddSubcommand(const CommandInfo& info,
llvm::function_ref<auto(CommandBuilder&)->void> build)
-> void;

// Subcommands can be hidden from the help listing of their parents with
// this setting. Hiding a subcommand doesn't disable its own help, it just
// removes it from the listing.
void HelpHidden(bool is_help_hidden);
auto HelpHidden(bool is_help_hidden) -> void;

// Exactly one of these three should be called to select and configure the
// kind of the built command.
void RequiresSubcommand();
void Do(ActionT action);
void Meta(ActionT meta_action);
auto RequiresSubcommand() -> void;
auto Do(ActionT action) -> void;
auto Meta(ActionT meta_action) -> void;

private:
friend Parser;
Expand All @@ -632,9 +640,9 @@ class CommandBuilder {
explicit CommandBuilder(Command* command, MetaPrinter* meta_printer);

auto AddArgImpl(const ArgInfo& info, ArgKind kind) -> Arg*;
void AddPositionalArgImpl(const ArgInfo& info, ArgKind kind,
llvm::function_ref<void(Arg&)> build);
void Finalize();
auto AddPositionalArgImpl(const ArgInfo& info, ArgKind kind,
llvm::function_ref<auto(Arg&)->void> build) -> void;
auto Finalize() -> void;

Command* command_;
MetaPrinter* meta_printer_;
Expand All @@ -658,7 +666,7 @@ class CommandBuilder {
// to `out`.
auto Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args,
llvm::raw_ostream& out, CommandInfo command_info,
llvm::function_ref<void(CommandBuilder&)> build)
llvm::function_ref<auto(CommandBuilder&)->void> build)
-> ErrorOr<ParseResult>;

// Implementation details only below.
Expand All @@ -667,8 +675,8 @@ auto Parse(llvm::ArrayRef<llvm::StringRef> unparsed_args,
struct Arg {
using Kind = ArgKind;
using ValueActionT =
std::function<bool(const Arg& arg, llvm::StringRef value_string)>;
using DefaultActionT = std::function<void(const Arg& arg)>;
std::function<auto(const Arg& arg, llvm::StringRef value_string)->bool>;
using DefaultActionT = std::function<auto(const Arg& arg)->void>;

explicit Arg(ArgInfo info);
~Arg();
Expand Down Expand Up @@ -733,7 +741,7 @@ struct Command {
};

template <typename T>
void ArgBuilder::MetaAction(T action) {
auto ArgBuilder::MetaAction(T action) -> void {
CARBON_CHECK(!arg_->meta_action, "Cannot set a meta action twice!");
arg_->meta_action = std::move(action);
}
Expand All @@ -757,7 +765,8 @@ auto OneOfArgBuilder::OneOfValue(llvm::StringRef str, T value)
}

template <typename T, typename U, size_t N>
void OneOfArgBuilder::SetOneOf(const OneOfValueT<U> (&values)[N], T* result) {
auto OneOfArgBuilder::SetOneOf(const OneOfValueT<U> (&values)[N], T* result)
-> void {
static_assert(N > 0, "Must include at least one value.");
arg()->is_append = false;
OneOfImpl(
Expand All @@ -766,8 +775,8 @@ void OneOfArgBuilder::SetOneOf(const OneOfValueT<U> (&values)[N], T* result) {
}

template <typename T, typename U, size_t N>
void OneOfArgBuilder::AppendOneOf(const OneOfValueT<U> (&values)[N],
llvm::SmallVectorImpl<T>* sequence) {
auto OneOfArgBuilder::AppendOneOf(const OneOfValueT<U> (&values)[N],
llvm::SmallVectorImpl<T>* sequence) -> void {
static_assert(N > 0, "Must include at least one value.");
arg()->is_append = true;
OneOfImpl(
Expand All @@ -787,9 +796,10 @@ void OneOfArgBuilder::AppendOneOf(const OneOfValueT<U> (&values)[N],
// lambdas that do the type-aware operations and storing those into type-erased
// function objects.
template <typename U, size_t N, typename MatchT, size_t... Indices>
void OneOfArgBuilder::OneOfImpl(const OneOfValueT<U> (&input_values)[N],
auto OneOfArgBuilder::OneOfImpl(const OneOfValueT<U> (&input_values)[N],
MatchT match,
std::index_sequence<Indices...> /*indices*/) {
std::index_sequence<Indices...> /*indices*/)
-> void {
std::array<llvm::StringRef, N> value_strings = {input_values[Indices].str...};
std::array<U, N> values = {input_values[Indices].value...};

Expand Down
6 changes: 3 additions & 3 deletions common/command_line_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -437,9 +437,9 @@ TEST(ArgParserTest, PositionalAppending) {
EXPECT_THAT(strings3, ElementsAre(StrEq("e"), StrEq("f")));
}

static auto ParseOneOfOption(llvm::ArrayRef<llvm::StringRef> args,
llvm::raw_ostream& s,
llvm::function_ref<void(OneOfArgBuilder&)> build)
static auto ParseOneOfOption(
llvm::ArrayRef<llvm::StringRef> args, llvm::raw_ostream& s,
llvm::function_ref<auto(OneOfArgBuilder&)->void> build)
-> ErrorOr<ParseResult> {
return Parse(args, s, TestCommandInfo, [&](auto& b) {
b.AddOneOfOption({.name = "option"}, build);
Expand Down
4 changes: 2 additions & 2 deletions common/error.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace Carbon {
// using `ErrorOr<Success>` and `return Success();` if no value needs to be
// returned.
struct Success : public Printable<Success> {
void Print(llvm::raw_ostream& out) const { out << "Success"; }
auto Print(llvm::raw_ostream& out) const -> void { out << "Success"; }
};

// Tracks an error message.
Expand Down Expand Up @@ -50,7 +50,7 @@ class [[nodiscard]] Error : public Printable<Error> {
}

// Prints the error string.
void Print(llvm::raw_ostream& out) const {
auto Print(llvm::raw_ostream& out) const -> void {
if (!location().empty()) {
out << location() << ": ";
}
Expand Down
2 changes: 1 addition & 1 deletion common/hashing_benchmark.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ struct LLVMHashBench : HashBenchBase {
};

template <typename Values, typename Hasher>
void BM_LatencyHash(benchmark::State& state) {
auto BM_LatencyHash(benchmark::State& state) -> void {
uint64_t x = 13;
Values v;
Hasher h;
Expand Down
24 changes: 12 additions & 12 deletions common/map.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ class MapView

// Run the provided callback for every key and value in the map.
template <typename CallbackT>
void ForEach(CallbackT callback)
auto ForEach(CallbackT callback) -> void
requires(std::invocable<CallbackT, KeyT&, ValueT&>);

// This routine is relatively inefficient and only intended for use in
Expand Down Expand Up @@ -224,7 +224,7 @@ class MapBase : protected RawHashtable::BaseImpl<InputKeyT, InputValueT,

// Convenience forwarder to the view type.
template <typename CallbackT>
void ForEach(CallbackT callback) const
auto ForEach(CallbackT callback) const -> void
requires(std::invocable<CallbackT, KeyT&, ValueT&>)
{
return ViewT(*this).ForEach(callback);
Expand Down Expand Up @@ -345,7 +345,7 @@ class MapBase : protected RawHashtable::BaseImpl<InputKeyT, InputValueT,

// Clear all key/value pairs from the map but leave the underlying hashtable
// allocated and in place.
void Clear();
auto Clear() -> void;

protected:
using ImplT::ImplT;
Expand Down Expand Up @@ -390,7 +390,7 @@ class Map : public RawHashtable::TableImpl<

// Reset the entire state of the hashtable to as it was when constructed,
// throwing away any intervening allocations.
void Reset();
auto Reset() -> void;
};

template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
Expand Down Expand Up @@ -418,8 +418,8 @@ auto MapView<InputKeyT, InputValueT, InputKeyContextT>::operator[](

template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
template <typename CallbackT>
void MapView<InputKeyT, InputValueT, InputKeyContextT>::ForEach(
CallbackT callback)
auto MapView<InputKeyT, InputValueT, InputKeyContextT>::ForEach(
CallbackT callback) -> void
requires(std::invocable<CallbackT, KeyT&, ValueT&>)
{
this->ForEachEntry(
Expand Down Expand Up @@ -551,14 +551,14 @@ MapBase<InputKeyT, InputValueT, InputKeyContextT>::Update(
}

template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
void MapBase<InputKeyT, InputValueT, InputKeyContextT>::GrowToAllocSize(
ssize_t target_alloc_size, KeyContextT key_context) {
auto MapBase<InputKeyT, InputValueT, InputKeyContextT>::GrowToAllocSize(
ssize_t target_alloc_size, KeyContextT key_context) -> void {
this->GrowToAllocSizeImpl(target_alloc_size, key_context);
}

template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
void MapBase<InputKeyT, InputValueT, InputKeyContextT>::GrowForInsertCount(
ssize_t count, KeyContextT key_context) {
auto MapBase<InputKeyT, InputValueT, InputKeyContextT>::GrowForInsertCount(
ssize_t count, KeyContextT key_context) -> void {
this->GrowForInsertCountImpl(count, key_context);
}

Expand All @@ -570,13 +570,13 @@ auto MapBase<InputKeyT, InputValueT, InputKeyContextT>::Erase(
}

template <typename InputKeyT, typename InputValueT, typename InputKeyContextT>
void MapBase<InputKeyT, InputValueT, InputKeyContextT>::Clear() {
auto MapBase<InputKeyT, InputValueT, InputKeyContextT>::Clear() -> void {
this->ClearImpl();
}

template <typename InputKeyT, typename InputValueT, ssize_t SmallSize,
typename InputKeyContextT>
void Map<InputKeyT, InputValueT, SmallSize, InputKeyContextT>::Reset() {
auto Map<InputKeyT, InputValueT, SmallSize, InputKeyContextT>::Reset() -> void {
this->ResetImpl();
}

Expand Down
Loading

0 comments on commit 2fef1cb

Please sign in to comment.