Peach Exceptions by joren-dev · Pull Request #1 · joren-dev/Peach-Library

joren-dev · 2020-11-19T01:03:39Z

Overview

Changes:

Introduces exception structure

Usage

#include <fstream>
#include <iostream>
#include <peach_library/exceptions/exception.hpp>

namespace peach
{
    enum class ExceptionTypes
    {
        kOutOfLine
    };
}

int main()
{

    peach::PrefixManager::set_default_prefix("log");

    peach::Prefix<peach::ExceptionTypes::kOutOfLine>::get().set_prefix("out_of_line");

    try
    {
        throw peach::PeachException<peach::ExceptionTypes::kOutOfLine>(peach::SL, "Something on", 23, "faulted");
    }
    catch (const peach::PeachException<peach::ExceptionTypes::kOutOfLine>& ec)
    {
        std::ofstream obj("error_log.txt");
        std::ostringstream ss;

        obj << ec;
        ss << ec;

        ec.print_to_console();
        std::cout << '\n' << ss.str();
    }

    return 0;
}

Output

// File output:
[out_of_line][03:27:30] at (line:file) 23 : C:\...\main.cpp ->
		{ asd 23 as  }

// Console output: 
out_of_line( " [out_of_line][03:27:30] at (line:file) 23 : C:\...\main.cpp ->
                { asd 23 as  } " )

[out_of_line][03:27:30] at (line:file) 23 : C:\...\main.cpp ->
                { asd 23 as  }

Note that paths here above are hidden with the three dots

Any changes on files besides the ones directly connected to your feature/change (emtpy if none):

none

Additional Notes:

none

Tests:

Test feature thorougly
Compiles w/ clang 11
Formatted according to the .clang-format

Extra

Add explanation/documentation on wiki/readme page

…erload for what()

joren-dev · 2022-02-10T02:31:36Z

      "ctestCommandArgs": "",
-      "inheritEnvironments": [ "clang_cl_x64_x64" ],
-      "variables": []
+      "inheritEnvironments": [ "msvc_x64_x64" ]


Im pretty sure these files should be hidden?

I would suggest to either remove this file from git or make is obvious that it is a IDE dependent file.

joren-dev · 2022-02-10T02:32:56Z

+
+namespace peach
+{
+  enum class ExceptionTypes;


Since the ::detail part was removed in this file, this code can now be moved into the same scope as the rest

joren-dev · 2022-02-10T02:33:42Z

+                                                     std::forward< Tys >( args )... ) }
+
+    {
+      m_err_msg = this->what( );


are we sure this is legal?

Yes, this should be fine, as we do not overwrite what() and at this point the base std::runtime_error is fully constructed.
You should be able to omit the this->:

Suggested change

m_err_msg = this->what( );

m_err_msg = what( );

joren-dev · 2022-02-10T02:35:40Z

+    void print_to_console( ) const noexcept
+    {
+      // TODO: possibly determine if compiled application has a console available!
+      std::cerr << m_prefix << R"(( " )" << m_err_msg.substr( 0, m_err_msg.size( ) - 2 )
+                << R"( " ))" << '\n';
+    }


I would actually want to get rid of this, they can already get this precise message in a string, the user can just std::cout or print however the user seems fit, saves us a lot of trouble ensuring the constraints for this method

joren-dev · 2022-02-10T02:36:12Z

-    ss << " }\n\n";
+    const auto put_time = std::put_time( &current_time, "[%T]" );
+
+    if ( line_num == constants::kNoSourceLocation && file_name.empty( ) )


this is not the most elegant way, any alternatives?

Isnt it enough to check for constants::kNoSourceLocation?

Also you could split the passing into the stringstream into multiple lines to avoid duplicate code in the if and else body.

Its enough to check for constants::kNoSourceLocation but in case value of constants::kNoSourceLocation(-1) is passed but the file name isnt empty, then it should just assume it has the std::source_location data dont you recon?

The only time we dont want to use it is when we pass (constants::kNoSourceLocation, "") to format_error, any other case def isnt using that ctor - might as well be waterproof

As for the duplicate code, theres no clean way to really do what you suggest - as both versions differ in quite some points somewhere in the middle of the stringstream. Do you have a suggestion on how to do it cleanly?

something like this should work:

ss << '[' << prefix << ']' << put_time; if ( line_num == constants::kNoSourceLocation && file_name.empty( ) ) { ss << " -> { "; } else { ss << '[' << prefix << ']' << put_time << " at (line:file) " << line_num << " : " << file_name << " ->\n\t\t{ "; } ( ( ss << std::forward< Tys >( args ) << ' ' ), ... ); ss << " }\n\n";

Nerlant · 2022-02-10T09:55:59Z

+.vs/
+out/
+/out
+/build


Maybe reorder this to look nicer and remove the duplicate build folder

Nerlant · 2022-02-10T09:57:43Z

      "ctestCommandArgs": "",
-      "inheritEnvironments": [ "clang_cl_x64_x64" ],
-      "variables": []
+      "inheritEnvironments": [ "msvc_x64_x64" ]


I would suggest to either remove this file from git or make is obvious that it is a IDE dependent file.

Nerlant · 2022-02-10T10:01:53Z

+    }
+    // clang-format on
+
+    inline static std::string m_default_prefix = { "LOG" };


this might be protected

Nerlant · 2022-02-10T10:13:24Z

+  // Short info:
+  // * Provides general way to set default prefix
+  // Link docu:
+  class PrefixManager


Creating instances of this does not really make any sense. So either the constructor should be protected (for inheritance to work), or we replace the class with a namespace that stores the default prefix (preferred by me).

Nerlant · 2022-02-10T10:13:59Z

+    }
+    // clang-format on
+
+    std::string m_prefix;


this should be private.

Nerlant · 2022-02-10T10:31:24Z

+  // * The method what() isnt templated in the base class, so we cannot override what()
+  // * First ctor handles the case when std::source_location information is wanted, second does it without
+  // Link docu:
+  template< ExceptionTypes Val > class PeachException : virtual public detail::BaseException


I see no need to use virtual inheritance here.

Nerlant · 2022-02-10T10:32:52Z

-    ss << " }\n\n";
+    const auto put_time = std::put_time( &current_time, "[%T]" );
+
+    if ( line_num == constants::kNoSourceLocation && file_name.empty( ) )


Isnt it enough to check for constants::kNoSourceLocation?

Also you could split the passing into the stringstream into multiple lines to avoid duplicate code in the if and else body.

Nerlant · 2022-02-10T10:35:21Z

+    const time_t t { std::time( nullptr ) };
+    tm current_time { };
+    localtime_s( &current_time, &t );


IIRC localtime_s does not exist on Linux. So when ensuring Linux compatibility we will need to check for the platform we compile for. Therefore i would vote to create a function that returns the time in which we can do the platform dependent stuff .

Nerlant · 2022-02-10T10:38:39Z

+      ss << " }\n\n";
+    }
+
+    return std::string { ss.str( ) };


Is the wrapping into the std::string really necessary? When the return type is std::string, it should be moved/copied anyway.

rebase excption structure

cc341b3

joren-dev added the work in progress label Nov 19, 2020

joren-dev requested a review from Nerlant November 19, 2020 01:03

joren-dev self-assigned this Nov 19, 2020

joren-dev marked this pull request as draft November 19, 2020 01:06

joren-dev added 5 commits November 19, 2020 02:22

fix initialization order issue

99852cd

change clang-format setting, format code, adapt format message

004f962

Merge branch 'master' into exceptions

25cdb5e

add security against using namespace std

3589d83

implement output_to_file, clean code

7fac1d4

joren-dev marked this pull request as ready for review November 19, 2020 16:31

Yuhanun reviewed Nov 19, 2020

View reviewed changes

joren-dev added 3 commits November 20, 2020 02:29

solve conversations, change exception output

79cab40

add noexcept to print_to_console and change std::cerr output

557e6e9

change console message

d08f6d7

Nerlant requested changes Nov 20, 2020

View reviewed changes

joren-dev and others added 7 commits October 28, 2021 09:56

change overload to work with all streams, remove unclear code, add ov…

24a3f54

…erload for what()

test

1014e65

remove build folders

f9498f7

remove build folders

c8124df

change method names, clean code, fix possible security flaw

2abcd1f

restructure library to support cmake install

1c7bcf7

fully rework ideology of exceptions

44923fa

joren-dev requested a review from Nerlant February 10, 2022 02:25

joren-dev commented Feb 10, 2022

View reviewed changes

Nerlant requested changes Feb 10, 2022

View reviewed changes

Conversation

joren-dev commented Nov 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Changes:

Usage

Output

Any changes on files besides the ones directly connected to your feature/change (emtpy if none):

Additional Notes:

Tests:

Extra

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joren-dev Feb 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

joren-dev commented Nov 19, 2020 •

edited

Loading

joren-dev Feb 12, 2022 •

edited

Loading