Skip to content

Commit

Permalink
Fix clang-tidy issues in common and testing (carbon-language#3470)
Browse files Browse the repository at this point in the history
Choosing to make the constructor explicit in the test, rather than
NOLINT, because it seems to better reflect how our code is usually
written (and may be more likely to trip an issue).
  • Loading branch information
jonmeow committed Dec 7, 2023
1 parent 2e29b48 commit bd0ef62
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 8 deletions.
7 changes: 5 additions & 2 deletions common/struct_reflection.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,11 @@ namespace Internal {
template <typename T>
struct AnyField {
template <typename FieldT>
// NOLINTNEXTLINE(google-explicit-constructor)
operator FieldT&() const;

template <typename FieldT>
// NOLINTNEXTLINE(google-explicit-constructor)
operator FieldT&&() const;

// Don't allow conversion to T itself. This ensures we don't match against a
Expand All @@ -52,11 +55,11 @@ struct AnyField {

// Detector for whether we can list-initialize T from the given list of fields.
template <typename T, typename... Fields>
constexpr bool CanListInitialize(decltype(T{Fields()...})*) {
constexpr auto CanListInitialize(decltype(T{Fields()...})* /*unused*/) -> bool {
return true;
}
template <typename T, typename... Fields>
constexpr bool CanListInitialize(...) {
constexpr auto CanListInitialize(...) -> bool {
return false;
}

Expand Down
11 changes: 6 additions & 5 deletions common/struct_reflection_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ struct ReferenceField {
};

struct NoDefaultConstructor {
NoDefaultConstructor(int n) : v(n) {}
explicit NoDefaultConstructor(int n) : v(n) {}
int v;
};

Expand All @@ -42,16 +42,16 @@ TEST(StructReflectionTest, CanListInitialize) {
{
using Type = OneField;
using Field = Internal::AnyField<Type>;
static_assert(Internal::CanListInitialize<Type>(0));
static_assert(Internal::CanListInitialize<Type, Field>(0));
static_assert(Internal::CanListInitialize<Type>(nullptr));
static_assert(Internal::CanListInitialize<Type, Field>(nullptr));
static_assert(!Internal::CanListInitialize<Type, Field, Field>(0));
}

{
using Type = OneFieldNoDefaultConstructor;
using Field = Internal::AnyField<Type>;
static_assert(!Internal::CanListInitialize<Type>(0));
static_assert(Internal::CanListInitialize<Type, Field>(0));
static_assert(Internal::CanListInitialize<Type, Field>(nullptr));
static_assert(!Internal::CanListInitialize<Type, Field, Field>(0));
}
}
Expand Down Expand Up @@ -82,7 +82,8 @@ TEST(StructReflectionTest, TwoField) {

TEST(StructReflectionTest, NoDefaultConstructor) {
std::tuple<NoDefaultConstructor, NoDefaultConstructor> fields =
AsTuple(TwoFieldsNoDefaultConstructor{.x = 1, .y = 2});
AsTuple(TwoFieldsNoDefaultConstructor{.x = NoDefaultConstructor(1),
.y = NoDefaultConstructor(2)});
EXPECT_EQ(std::get<0>(fields).v, 1);
EXPECT_EQ(std::get<1>(fields).v, 2);
}
Expand Down
2 changes: 1 addition & 1 deletion testing/file_test/line.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class FileTestLineBase : public Printable<FileTestLineBase> {
public:
explicit FileTestLineBase(int file_number, int line_number)
: file_number_(file_number), line_number_(line_number) {}
virtual ~FileTestLineBase() {}
virtual ~FileTestLineBase() = default;

// Prints the autoupdated line.
virtual auto Print(llvm::raw_ostream& out) const -> void = 0;
Expand Down

0 comments on commit bd0ef62

Please sign in to comment.