diff --git a/common/check.h b/common/check.h index fa59a9880944..34732131b6c2 100644 --- a/common/check.h +++ b/common/check.h @@ -9,78 +9,6 @@ #include "llvm/Support/Signals.h" #include "llvm/Support/raw_ostream.h" -namespace Carbon { -namespace Internal { - -// Wraps a stream and exiting for fatal errors. Should only be used by the -// macros below. -class ExitingStream { - public: - // A tag type that renders as ": " in an ExitingStream, but only if it is - // followed by additional output. Otherwise, it renders as "". Primarily used - // when building macros around these streams. - struct AddSeparator {}; - - // Internal type used in macros to dispatch to the `operator|` overload below. - struct Helper {}; - - [[noreturn]] ~ExitingStream() { - llvm_unreachable( - "Exiting streams should only be constructed with the below macros that " - "ensure the special operator| exits the program prior to their " - "destruction!"); - } - - // Indicates that the program is exiting due to a bug in the program, rather - // than, e.g., invalid input. - ExitingStream& TreatAsBug() { - treat_as_bug_ = true; - return *this; - } - - // If the bool cast occurs, it's because the condition is false. This supports - // && short-circuiting the creation of ExitingStream. - explicit operator bool() const { return true; } - - // Forward output to llvm::errs. - template - ExitingStream& operator<<(const T& message) { - if (separator_) { - llvm::errs() << ": "; - separator_ = false; - } - llvm::errs() << message; - return *this; - } - - ExitingStream& operator<<(AddSeparator /*unused*/) { - separator_ = true; - return *this; - } - - // Low-precedence binary operator overload used in macros below to flush the - // output and exit the program. We do this in a binary operator rather than - // the destructor to ensure good debug info and backtraces for errors. - [[noreturn]] friend auto operator|(Helper /*unused*/, ExitingStream& rhs) { - // Finish with a newline. - llvm::errs() << "\n"; - if (rhs.treat_as_bug_) { - std::abort(); - } else { - std::exit(-1); - } - } - - private: - // Whether a separator should be printed if << is used again. - bool separator_ = false; - - // Whether the program is exiting due to a bug. - bool treat_as_bug_ = false; -}; - -} // namespace Internal - // Raw exiting stream. This should be used when building other forms of exiting // macros like those below. It evaluates to a temporary `ExitingStream` object // that can be manipulated, streamed into, and then will exit the program. @@ -109,6 +37,75 @@ class ExitingStream { RAW_EXITING_STREAM().TreatAsBug() \ << "FATAL failure at " << __FILE__ << ":" << __LINE__ << ": " -} // namespace Carbon +namespace Carbon::Internal { + +// Wraps a stream and exiting for fatal errors. Should only be used by the +// macros below. +class ExitingStream { + public: + // A tag type that renders as ": " in an ExitingStream, but only if it is + // followed by additional output. Otherwise, it renders as "". Primarily used + // when building macros around these streams. + struct AddSeparator {}; + + // Internal type used in macros to dispatch to the `operator|` overload below. + struct Helper {}; + + [[noreturn]] ~ExitingStream() { + llvm_unreachable( + "Exiting streams should only be constructed with the below macros that " + "ensure the special operator| exits the program prior to their " + "destruction!"); + } + + // Indicates that the program is exiting due to a bug in the program, rather + // than, e.g., invalid input. + auto TreatAsBug() -> ExitingStream& { + treat_as_bug_ = true; + return *this; + } + + // If the bool cast occurs, it's because the condition is false. This supports + // && short-circuiting the creation of ExitingStream. + explicit operator bool() const { return true; } + + // Forward output to llvm::errs. + template + auto operator<<(const T& message) -> ExitingStream& { + if (separator_) { + llvm::errs() << ": "; + separator_ = false; + } + llvm::errs() << message; + return *this; + } + + auto operator<<(AddSeparator /*unused*/) -> ExitingStream& { + separator_ = true; + return *this; + } + + // Low-precedence binary operator overload used in macros below to flush the + // output and exit the program. We do this in a binary operator rather than + // the destructor to ensure good debug info and backtraces for errors. + [[noreturn]] friend auto operator|(Helper /*unused*/, ExitingStream& rhs) { + // Finish with a newline. + llvm::errs() << "\n"; + if (rhs.treat_as_bug_) { + std::abort(); + } else { + std::exit(-1); + } + } + + private: + // Whether a separator should be printed if << is used again. + bool separator_ = false; + + // Whether the program is exiting due to a bug. + bool treat_as_bug_ = false; +}; + +} // namespace Carbon::Internal #endif // COMMON_CHECK_H_ diff --git a/common/indirect_value.h b/common/indirect_value.h index d534ecad9e17..d30b0cbdc6e0 100644 --- a/common/indirect_value.h +++ b/common/indirect_value.h @@ -48,6 +48,7 @@ class IndirectValue { IndirectValue() : value_(std::make_unique()) {} // Initializes the underlying T object as if by `T(std::move(value))`. + // NOLINTNEXTLINE(google-explicit-constructor): Implicit constructor. IndirectValue(T value) : value_(std::make_unique(std::move(value))) {} // TODO(geoffromer): consider defining implicit conversions from @@ -56,7 +57,7 @@ class IndirectValue { IndirectValue(const IndirectValue& other) : value_(std::make_unique(*other)) {} - IndirectValue(IndirectValue&& other) + IndirectValue(IndirectValue&& other) noexcept : value_(std::make_unique(std::move(*other))) {} auto operator=(const IndirectValue& other) -> IndirectValue& { @@ -64,7 +65,7 @@ class IndirectValue { return *this; } - auto operator=(IndirectValue&& other) -> IndirectValue& { + auto operator=(IndirectValue&& other) noexcept -> IndirectValue& { *value_ = std::move(*other.value_); return *this; } @@ -91,7 +92,7 @@ class IndirectValue { -> IndirectValue>; template - IndirectValue(std::unique_ptr value) : value_(std::move(value)) {} + explicit IndirectValue(std::unique_ptr value) : value_(std::move(value)) {} const std::unique_ptr value_; }; diff --git a/common/indirect_value_test.cpp b/common/indirect_value_test.cpp index 1c5673e00eb7..b9f903e69cc7 100644 --- a/common/indirect_value_test.cpp +++ b/common/indirect_value_test.cpp @@ -26,7 +26,7 @@ TEST(IndirectValueTest, MutableAccess) { } struct NonMovable { - NonMovable(int i) : i(i) {} + explicit NonMovable(int i) : i(i) {} NonMovable(NonMovable&&) = delete; auto operator=(NonMovable&&) -> NonMovable& = delete; @@ -39,7 +39,7 @@ TEST(IndirectValueTest, Create) { EXPECT_EQ(v->i, 42); } -const int& GetIntReference() { +auto GetIntReference() -> const int& { static int i = 42; return i; } @@ -54,15 +54,15 @@ TEST(IndirectValueTest, CreateWithDecay) { // member function (if any) caused it to reach its present value. struct TestValue { TestValue() : state("default constructed") {} - TestValue(const TestValue& rhs) : state("copy constructed") {} - TestValue(TestValue&& other) : state("move constructed") { + TestValue(const TestValue& /*rhs*/) : state("copy constructed") {} + TestValue(TestValue&& other) noexcept : state("move constructed") { other.state = "move constructed from"; } - TestValue& operator=(const TestValue&) { + auto operator=(const TestValue&) noexcept -> TestValue& { state = "copy assigned"; return *this; } - TestValue& operator=(TestValue&& other) { + auto operator=(TestValue&& other) noexcept -> TestValue& { state = "move assigned"; other.state = "move assigned from"; return *this; @@ -101,6 +101,8 @@ TEST(IndirectValueTest, CopyAssign) { TEST(IndirectValueTest, MoveConstruct) { IndirectValue v1; auto v2 = std::move(v1); + // While not entirely safe, the `v1->state` access tests move behavior. + // NOLINTNEXTLINE(bugprone-use-after-move) EXPECT_EQ(v1->state, "move constructed from"); EXPECT_EQ(v2->state, "move constructed"); } @@ -109,6 +111,8 @@ TEST(IndirectValueTest, MoveAssign) { IndirectValue v1; IndirectValue v2; v2 = std::move(v1); + // While not entirely safe, the `v1->state` access tests move behavior. + // NOLINTNEXTLINE(bugprone-use-after-move) EXPECT_EQ(v1->state, "move assigned from"); EXPECT_EQ(v2->state, "move assigned"); }