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.

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

Simplify C++ exceptions #3380

Merged
merged 7 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
119 changes: 28 additions & 91 deletions cpp/include/Ice/Exception.h
Original file line number Diff line number Diff line change
@@ -1,112 +1,49 @@
//
// Copyright (c) ZeroC, Inc. All rights reserved.
//
// Copyright (c) ZeroC, Inc.

#ifndef ICE_EXCEPTION_H
#define ICE_EXCEPTION_H

#include "Config.h"
#include "ValueF.h"

#include <exception>
#include <memory>
#include <ostream>
#include <string>
#include <vector>

namespace Ice
{
/**
* Abstract base class for all Ice exceptions. It has only two derived classes: LocalException and UserException.
* \headerfile Ice/Ice.h
*/
/// Abstract base class for all Ice exceptions. It has only two derived classes: LocalException and UserException.
/// \headerfile Ice/Ice.h
class ICE_API Exception : public std::exception
{
public:
/**
* Constructs an exception with a default message.
* @param file The file where this exception is constructed. This C string is not copied.
* @param line The line where this exception is constructed.
*/
Exception(const char* file, int line) noexcept;

/**
* Constructs an exception.
* @param file The file where this exception is constructed. This C string is not copied.
* @param line The line where this exception is constructed.
* @param message The error message adopted by this exception and returned by what().
*/
Exception(const char* file, int line, std::string message);

/**
* Copy constructor.
* @param other The exception to copy.
*/
Exception(const Exception& other) noexcept = default;

/**
* Assignment operator.
* @param rhs The exception to assign.
*/
Exception& operator=(const Exception& rhs) noexcept = default;

/**
* Returns the error message of this exception.
* @return The error message.
*/
[[nodiscard]] const char* what() const noexcept override;

/**
* Returns the type ID of this exception. This corresponds to the Slice type ID for Slice-defined exceptions,
* and to a similar fully scoped name for other exceptions. For example "::Ice::CommunicatorDestroyedException".
* @return The type ID of this exception
*/
Exception() noexcept = default;
Exception(const Exception&) noexcept = default;
Exception& operator=(const Exception&) noexcept = default;

// Need out-of-line virtual function to avoid weak vtable, which in turn requires the default constructor,
// copy constructor, and copy assignment operator to be declared explicitly.
Copy link
Member Author

Choose a reason for hiding this comment

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

Weak vtables break dynamic_cast and catch(const XxxException&), and it's typically hard to find since we see these errors only at runtime. This particular weak vtable breaks the Ice/background test when it catches const Ice::Exception& on macos.

~Exception() override;

/// Returns the type ID of this exception. This corresponds to the Slice type ID for Slice-defined exceptions,
/// and to a similar fully scoped name for other exceptions. For example
/// "::Ice::CommunicatorDestroyedException".
/// @return The type ID of this exception
[[nodiscard]] virtual const char* ice_id() const noexcept = 0;

/**
* Outputs a description of this exception to a stream. This function is called by operator<<(std::ostream&,
* const Ice::Exception&). The default implementation outputs ice_id(). The application can override the
* ice_print of a user exception to produce a more detailed description, with typically the ice_id() plus
* additional information.
* @param os The output stream.
*/
virtual void ice_print(std::ostream& os) const { os << ice_id(); }

/**
* Returns the name of the file where this exception was constructed.
* @return The file name.
*/
[[nodiscard]] const char* ice_file() const noexcept;

/**
* Returns the line number where this exception was constructed.
* @return The line number.
*/
[[nodiscard]] int ice_line() const noexcept;

/**
* Returns the stack trace at the point this exception was constructed.
* @return The stack trace as a string, or an empty string if stack trace collection is not enabled.
*/
[[nodiscard]] std::string ice_stackTrace() const;

/**
* Enables the collection of stack traces for exceptions. On Windows, calling this function more than once is
* useful to refresh the symbol module list; on other platforms, the second and subsequent calls have no effect.
*/
static void ice_enableStackTraceCollection();

private:
friend ICE_API std::ostream& operator<<(std::ostream&, const Exception&);

const char* _file; // can be nullptr
int _line; // not used when _file is nullptr
std::shared_ptr<std::string> _whatString; // shared storage for custom _what message.
const char* _what; // can be nullptr
std::shared_ptr<std::vector<void*>> _stackFrames; // shared storage for stack frames.
/// Outputs a description of this exception to a stream.
/// @param os The output stream.
virtual void ice_print(std::ostream& os) const = 0;
};

ICE_API std::ostream& operator<<(std::ostream&, const Exception&);
/// Outputs a description of an Ice exception to a stream by calling the ice_print member function of this
/// exception.
/// @param os The output stream.
/// @param exception The exception to describe.
/// @return The output stream.
inline std::ostream& operator<<(std::ostream& os, const Exception& exception)
{
exception.ice_print(os);
return os;
}
}

#endif
55 changes: 41 additions & 14 deletions cpp/include/Ice/LocalException.h
Original file line number Diff line number Diff line change
@@ -1,30 +1,57 @@
//
// Copyright (c) ZeroC, Inc. All rights reserved.
//
// Copyright (c) ZeroC, Inc.

#ifndef ICE_LOCAL_EXCEPTION_H
#define ICE_LOCAL_EXCEPTION_H

#include "Exception.h"

#include <memory>
#include <string>
#include <vector>

namespace Ice
{
/**
* Base class for all Ice exceptions not defined in Slice.
* \headerfile Ice/Ice.h
*/
/// Base class for all Ice exceptions not defined in Slice.
/// \headerfile Ice/Ice.h
class ICE_API LocalException : public Exception
{
public:
/**
* Constructs a local exception.
* @param file The file where this exception is constructed. This C string is not copied.
* @param line The line where this exception is constructed.
* @param message The error message adopted by this exception and returned by what().
*/
LocalException(const char* file, int line, std::string message) : Exception(file, line, std::move(message)) {}
/// Constructs a local exception.
/// @param file The file where this exception is constructed. This C string is not copied.
/// @param line The line where this exception is constructed.
/// @param message The error message adopted by this exception and returned by what().
LocalException(const char* file, int line, std::string message);

/// Gets the error message of this local Ice exception.
/// @return The error message.
[[nodiscard]] const char* what() const noexcept final;

Choose a reason for hiding this comment

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

I know that what is pretty standard C++ exception stuff, but should we carry over the doc-comment for it?

        /**
         * Returns the error message of this exception.
         * @return The error message.
         */
        [[nodiscard]] const char* what() const noexcept override;

void ice_print(std::ostream& os) const final;

[[nodiscard]] const char* ice_id() const noexcept override;

/// Gets the name of the file where this exception was constructed.
/// @return The file name.
[[nodiscard]] const char* ice_file() const noexcept;

/// Gets the line number where this exception was constructed.
/// @return The line number.
[[nodiscard]] int ice_line() const noexcept;

/// Gets the stack trace at the point this exception was constructed.
/// @return The stack trace as a string, or an empty string if stack trace collection is not enabled.
[[nodiscard]] std::string ice_stackTrace() const;

/// Enables the collection of stack traces for exceptions. On Windows, calling this function more than once is
/// useful to refresh the symbol module list; on other platforms, the second and subsequent calls have no
/// effect.
static void ice_enableStackTraceCollection();

private:
const char* _file;
int _line;
std::shared_ptr<std::string> _whatString; // shared storage for custom _what message.
std::shared_ptr<std::vector<void*>> _stackFrames; // shared storage for stack frames.
};
}

Expand Down
4 changes: 4 additions & 0 deletions cpp/include/Ice/LocalExceptions.h
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,10 @@ namespace Ice
{
}

RequestFailedException(const RequestFailedException&) noexcept = default;
RequestFailedException& operator=(const RequestFailedException&) noexcept = default;
~RequestFailedException() override; // to avoid weak vtable

private:
std::shared_ptr<Identity> _id;
std::shared_ptr<std::string> _facet;
Expand Down
27 changes: 10 additions & 17 deletions cpp/include/Ice/UserException.h
Original file line number Diff line number Diff line change
@@ -1,36 +1,29 @@
//
// Copyright (c) ZeroC, Inc. All rights reserved.
//
// Copyright (c) ZeroC, Inc.

#ifndef ICE_USER_EXCEPTION_H
#define ICE_USER_EXCEPTION_H

#include "Exception.h"

#include <string_view>

namespace Ice
{
class InputStream;
class OutputStream;

/**
* Abstract base class for all Ice exceptions defined in Slice.
* \headerfile Ice/Ice.h
*/
/// Abstract base class for all Ice exceptions defined in Slice.
/// \headerfile Ice/Ice.h
class ICE_API UserException : public Exception
{
public:
/**
* Default constructor. The file, line and what message are never set for user exceptions.
*/
UserException() : Exception(nullptr, 0) {}

/**
* Throws this exception.
*/
/// Throws this exception.
virtual void ice_throw() const = 0;

/// Gets the Slice type ID of this user exception.
/// @return The Slice type ID.
[[nodiscard]] const char* what() const noexcept final;

void ice_print(std::ostream& os) const override;

/// \cond STREAM
// _write and _read are virtual for the Python, Ruby etc. mappings.
virtual void _write(OutputStream*) const;
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/DataStorm/SessionI.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -766,7 +766,7 @@ SessionI::destroyImpl(const exception_ptr& ex)
{
rethrow_exception(ex);
}
catch (const Exception& e)
catch (const LocalException& e)
{
out << "\n:" << e.what() << "\n" << e.ice_stackTrace();
}
Expand Down
Loading
Loading