Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix VS static analyzer warnings. #1235

Merged
merged 2 commits into from
Oct 26, 2023
Merged

Fix VS static analyzer warnings. #1235

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
42a2949
Fix VS static analyzer warnings.
BillyONeal Oct 16, 2023
35351c2
Why did that compile on Windows?!
BillyONeal Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 13 additions & 9 deletions include/vcpkg/base/expected.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ namespace vcpkg
struct ExpectedHolder<T&>
{
ExpectedHolder() = delete;
ExpectedHolder(T& t) : t(&t) { }
ExpectedHolder(T& t) noexcept : t(&t) { }
ExpectedHolder(const ExpectedHolder&) = default;
ExpectedHolder& operator=(const ExpectedHolder&) = default;
using pointer = T*;
Expand Down Expand Up @@ -98,7 +98,9 @@ namespace vcpkg
{
}

ExpectedT(const ExpectedT& other) : value_is_error(other.value_is_error)
ExpectedT(const ExpectedT& other) noexcept(
std::is_nothrow_copy_constructible_v<Error>&& std::is_nothrow_copy_constructible_v<ExpectedHolder<T>>)
: value_is_error(other.value_is_error)
{
if (value_is_error)
{
Expand All @@ -110,7 +112,9 @@ namespace vcpkg
}
}

ExpectedT(ExpectedT&& other) : value_is_error(other.value_is_error)
ExpectedT(ExpectedT&& other) noexcept(
std::is_nothrow_move_constructible_v<Error>&& std::is_nothrow_move_constructible_v<ExpectedHolder<T>>)
: value_is_error(other.value_is_error)
{
if (value_is_error)
{
Expand Down Expand Up @@ -208,25 +212,25 @@ namespace vcpkg
return *m_t.get();
}

const T& value(const LineInfo& line_info) const&
const T& value(const LineInfo& line_info) const& noexcept
{
unreachable_if_error(line_info);
return *m_t.get();
}

T&& value(const LineInfo& line_info) &&
T&& value(const LineInfo& line_info) && noexcept
{
unreachable_if_error(line_info);
return std::move(*m_t.get());
}

const Error& error() const&
const Error& error() const& noexcept
{
unreachable_if_not_error(VCPKG_LINE_INFO);
return m_error;
}

Error&& error() &&
Error&& error() && noexcept
{
unreachable_if_not_error(VCPKG_LINE_INFO);
return std::move(m_error);
Expand Down Expand Up @@ -358,15 +362,15 @@ namespace vcpkg
}

private:
void exit_if_error(const LineInfo& line_info) const
void exit_if_error(const LineInfo& line_info) const noexcept
{
if (value_is_error)
{
Checks::msg_exit_with_message(line_info, error());
}
}

void unreachable_if_error(const LineInfo& line_info) const
void unreachable_if_error(const LineInfo& line_info) const noexcept
{
if (value_is_error)
{
Expand Down
120 changes: 65 additions & 55 deletions include/vcpkg/base/optional.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,17 @@
struct OptionalStorage
{
constexpr OptionalStorage() noexcept : m_is_present(false), m_inactive() { }
constexpr OptionalStorage(const T& t) : m_is_present(true), m_t(t) { }
constexpr OptionalStorage(T&& t) : m_is_present(true), m_t(std::move(t)) { }
constexpr OptionalStorage(const T& t) noexcept(std::is_nothrow_copy_constructible_v<T>)
: m_is_present(true), m_t(t)
{
}
constexpr OptionalStorage(T&& t) noexcept(std::is_nothrow_move_constructible_v<T>)
: m_is_present(true), m_t(std::move(t))
{
}
template<class U, class = std::enable_if_t<!std::is_reference_v<U>>>
explicit OptionalStorage(Optional<U>&& t) : m_is_present(false), m_inactive()
explicit OptionalStorage(Optional<U>&& t) noexcept(std::is_nothrow_constructible_v<T, U>)
: m_is_present(false), m_inactive()
{
if (auto p = t.get())
{
Expand All @@ -39,7 +46,8 @@
}
}
template<class U>
explicit OptionalStorage(const Optional<U>& t) : m_is_present(false), m_inactive()
explicit OptionalStorage(const Optional<U>& t) noexcept(std::is_nothrow_constructible_v<T, const U&>)
: m_is_present(false), m_inactive()
{
if (auto p = t.get())
{
Expand All @@ -48,25 +56,28 @@
}
}

~OptionalStorage() noexcept
~OptionalStorage()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why should this not be noexcept?

Copy link
Member Author

@BillyONeal BillyONeal Oct 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is a destructor, destructors are noexcept without being annotated as such (And we don't annotate other destructors)

Destructors should get noexcept annotations only if they are doing noexcept(false)

{
if (m_is_present) m_t.~T();
}

OptionalStorage(const OptionalStorage& o) : m_is_present(o.m_is_present), m_inactive()
OptionalStorage(const OptionalStorage& o) noexcept(std::is_nothrow_copy_constructible_v<T>)
: m_is_present(o.m_is_present), m_inactive()
{
if (m_is_present) new (&m_t) T(o.m_t);
}

OptionalStorage(OptionalStorage&& o) noexcept : m_is_present(o.m_is_present), m_inactive()
OptionalStorage(OptionalStorage&& o) noexcept(std::is_nothrow_move_constructible_v<T>)
: m_is_present(o.m_is_present), m_inactive()
{
if (m_is_present)
{
new (&m_t) T(std::move(o.m_t));
}
}

OptionalStorage& operator=(const OptionalStorage& o)
OptionalStorage& operator=(const OptionalStorage& o) noexcept(
std::is_nothrow_copy_constructible_v<T>&& std::is_nothrow_copy_assignable_v<T>)
{
if (m_is_present && o.m_is_present)
{
Expand All @@ -84,7 +95,7 @@
return *this;
}

OptionalStorage& operator=(OptionalStorage&& o) noexcept
OptionalStorage& operator=(OptionalStorage&& o) noexcept // enforces termination
{
if (m_is_present && o.m_is_present)
{
Expand All @@ -102,25 +113,25 @@
return *this;
}

constexpr bool has_value() const { return m_is_present; }
constexpr bool has_value() const noexcept { return m_is_present; }

const T& value() const { return this->m_t; }
T& value() { return this->m_t; }
const T& value() const noexcept { return this->m_t; }
T& value() noexcept { return this->m_t; }

const T* get() const& { return m_is_present ? &m_t : nullptr; }
T* get() & { return m_is_present ? &m_t : nullptr; }
const T* get() const& noexcept { return m_is_present ? &m_t : nullptr; }
T* get() & noexcept { return m_is_present ? &m_t : nullptr; }
const T* get() const&& = delete;
T* get() && = delete;

void destroy()
void destroy() noexcept // enforces termination
{
m_is_present = false;
m_t.~T();
m_inactive = '\0';
}

template<class... Args>
T& emplace(Args&&... args)
T& emplace(Args&&... args) noexcept(std::is_nothrow_constructible_v<T, Args...>)
{
if (m_is_present) destroy();
new (&m_t) T(static_cast<Args&&>(args)...);
Expand All @@ -141,22 +152,26 @@
struct OptionalStorage<T, false>
{
constexpr OptionalStorage() noexcept : m_is_present(false), m_inactive() { }
constexpr OptionalStorage(T&& t) : m_is_present(true), m_t(std::move(t)) { }
constexpr OptionalStorage(T&& t) noexcept(std::is_nothrow_move_constructible_v<T>)
: m_is_present(true), m_t(std::move(t))
{
}

~OptionalStorage() noexcept
~OptionalStorage()
{
if (m_is_present) m_t.~T();
}

OptionalStorage(OptionalStorage&& o) noexcept : m_is_present(o.m_is_present), m_inactive()
OptionalStorage(OptionalStorage&& o) noexcept(std::is_nothrow_move_constructible_v<T>)
: m_is_present(o.m_is_present), m_inactive()
{
if (m_is_present)
{
new (&m_t) T(std::move(o.m_t));
}
}

OptionalStorage& operator=(OptionalStorage&& o) noexcept
OptionalStorage& operator=(OptionalStorage&& o) noexcept // enforces termination
{
if (m_is_present && o.m_is_present)
{
Expand All @@ -174,26 +189,26 @@
return *this;
}

constexpr bool has_value() const { return m_is_present; }
constexpr bool has_value() const noexcept { return m_is_present; }

const T& value() const { return this->m_t; }
T& value() { return this->m_t; }
const T& value() const noexcept { return this->m_t; }
T& value() noexcept { return this->m_t; }

const T* get() const& { return m_is_present ? &m_t : nullptr; }
T* get() & { return m_is_present ? &m_t : nullptr; }
const T* get() const& noexcept { return m_is_present ? &m_t : nullptr; }
T* get() & noexcept { return m_is_present ? &m_t : nullptr; }
const T* get() const&& = delete;
T* get() && = delete;

template<class... Args>
T& emplace(Args&&... args)
T& emplace(Args&&... args) noexcept(std::is_nothrow_constructible<T, Args...>)

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (ubuntu-20.04, linux-ci) 

expected primary-expression before ‘)’ token

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction

Check failure on line 203 in include/vcpkg/base/optional.h

View workflow job for this annotation

GitHub Actions / builds / build (macos-12, macos-ci) 

expected '(' for function-style cast or type construction
{
if (m_is_present) destroy();
new (&m_t) T(static_cast<Args&&>(args)...);
m_is_present = true;
return m_t;
}

void destroy()
void destroy() noexcept
{
m_is_present = false;
m_t.~T();
Expand All @@ -213,22 +228,22 @@
struct OptionalStorage<T&, B>
{
constexpr OptionalStorage() noexcept : m_t(nullptr) { }
constexpr OptionalStorage(T& t) : m_t(&t) { }
constexpr OptionalStorage(Optional<T>& t) : m_t(t.get()) { }
constexpr OptionalStorage(T& t) noexcept : m_t(&t) { }
constexpr OptionalStorage(Optional<T>& t) noexcept : m_t(t.get()) { }

constexpr bool has_value() const { return m_t != nullptr; }
constexpr bool has_value() const noexcept { return m_t != nullptr; }

T& value() const { return *this->m_t; }
T& value() const noexcept { return *this->m_t; }

T& emplace(T& t)
T& emplace(T& t) noexcept
{
m_t = &t;
return *m_t;
}

T* get() const { return m_t; }
T* get() const noexcept { return m_t; }

void destroy() { m_t = nullptr; }
void destroy() noexcept { m_t = nullptr; }

private:
T* m_t;
Expand All @@ -238,25 +253,25 @@
struct OptionalStorage<const T&, B>
{
constexpr OptionalStorage() noexcept : m_t(nullptr) { }
constexpr OptionalStorage(const T& t) : m_t(&t) { }
constexpr OptionalStorage(const Optional<T>& t) : m_t(t.get()) { }
constexpr OptionalStorage(const Optional<const T>& t) : m_t(t.get()) { }
constexpr OptionalStorage(const T& t) noexcept : m_t(&t) { }
constexpr OptionalStorage(const Optional<T>& t) noexcept : m_t(t.get()) { }
constexpr OptionalStorage(const Optional<const T>& t) noexcept : m_t(t.get()) { }
constexpr OptionalStorage(Optional<T>&& t) = delete;
constexpr OptionalStorage(Optional<const T>&& t) = delete;

constexpr bool has_value() const { return m_t != nullptr; }
constexpr bool has_value() const noexcept { return m_t != nullptr; }

const T& value() const { return *this->m_t; }
const T& value() const noexcept { return *this->m_t; }

const T* get() const { return m_t; }
const T* get() const noexcept { return m_t; }

const T& emplace(const T& t)
const T& emplace(const T& t) noexcept
{
m_t = &t;
return *m_t;
}

void destroy() { m_t = nullptr; }
void destroy() noexcept { m_t = nullptr; }

private:
const T* m_t;
Expand All @@ -270,39 +285,40 @@
constexpr Optional() noexcept { }

// Constructors are intentionally implicit
constexpr Optional(NullOpt) { }
constexpr Optional(NullOpt) noexcept { }

template<class U,
std::enable_if_t<!std::is_same_v<std::decay_t<U>, Optional> &&
std::is_constructible_v<details::OptionalStorage<T>, U>,
int> = 0>
constexpr Optional(U&& t) : details::OptionalStorage<T>(static_cast<U&&>(t))
constexpr Optional(U&& t) noexcept(std::is_nothrow_constructible_v<details::OptionalStorage<T>, U>)
: details::OptionalStorage<T>(static_cast<U&&>(t))
{
}

using details::OptionalStorage<T>::emplace;
using details::OptionalStorage<T>::has_value;
using details::OptionalStorage<T>::get;

T&& value_or_exit(const LineInfo& line_info) &&
T&& value_or_exit(const LineInfo& line_info) && noexcept
{
Checks::check_exit(line_info, this->has_value(), "Value was null");
return std::move(this->value());
}

T& value_or_exit(const LineInfo& line_info) &
T& value_or_exit(const LineInfo& line_info) & noexcept
{
Checks::check_exit(line_info, this->has_value(), "Value was null");
return this->value();
}

const T& value_or_exit(const LineInfo& line_info) const&
const T& value_or_exit(const LineInfo& line_info) const& noexcept
{
Checks::check_exit(line_info, this->has_value(), "Value was null");
return this->value();
}

constexpr explicit operator bool() const { return this->has_value(); }
constexpr explicit operator bool() const noexcept { return this->has_value(); }

template<class U>
T value_or(U&& default_value) const&
Expand Down Expand Up @@ -371,7 +387,7 @@
return nullopt;
}

void clear()
void clear() noexcept
{
if (this->has_value())
{
Expand All @@ -396,12 +412,6 @@
friend bool operator!=(const Optional& lhs, const Optional& rhs) noexcept { return !(lhs == rhs); }
};

template<class U>
Optional<std::decay_t<U>> make_optional(U&& u)
{
return Optional<std::decay_t<U>>(std::forward<U>(u));
}

// these cannot be hidden friends, unfortunately
template<class T, class U>
auto operator==(const Optional<T>& lhs, const Optional<U>& rhs) -> decltype(*lhs.get() == *rhs.get())
Expand Down
1 change: 1 addition & 0 deletions src/vcpkg/base/system.process.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,6 +961,7 @@ namespace
RedirectedProcessInfo& operator=(const RedirectedProcessInfo&) = delete;
~RedirectedProcessInfo() = default;

VCPKG_MSVC_WARNING(suppress : 6262) // function uses 32k of stack
int wait_and_stream_output(int32_t debug_id,
const char* input,
DWORD input_size,
Expand Down
Loading