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

narrowing_error with more information #761

Closed
wants to merge 1 commit into from

Conversation

beinhaerter
Copy link
Contributor

  • added type trait is_streamable
  • added what to narrowing_error (not optimized for MSVC, just standard C++ stuff)
  • instead of "Unknown exception" we now get either "<type_from> to <type_to>" or "<type_from> to <type_to>"

This is done to provide as much debug information as possible in case of a narrowing_error exception.
I am not very experienced with type traits, so any idea how to do it better is very welcome. I took the code for is_streamable from https://stackoverflow.com/a/22759368/1023911.
I tested the code given this short program (output is given in the comments):

#include <iostream>
#include <typeinfo>

#define GSL_THROW_ON_CONTRACT_VIOLATION
#include <gsl/gsl_util>

struct INT {
	INT() :_x(0) {}
	INT(int x) :_x(x) {}
	operator int() noexcept { return _x; }

	int _x;
};

int main()
{
	try {
		gsl::narrow<char>(4200);
		gsl::narrow<char>(INT{ 4200 });
	}
	catch (const std::exception &e) {
		// Exception struct gsl::narrowing_error: Unknown exception
		// Exception struct gsl::narrowing_error: int 4200 to char
		// Exception struct gsl::narrowing_error: struct INT to char
		std::cout << "Exception " << typeid(e).name() << ": " << e.what() << std::endl;
	}

	return 0;
}

- added type trait is_streamable
- added what to narrowing_error (not optimized for MSVC, just standard C++ stuff)
- instead of "Unknown exception" we now get either "<type_from> <value> to <type_to>" or "<type_from> to <type_to>"
@annagrin
Copy link

annagrin commented Jan 17, 2019

@beinhaerter I am worried about increasing binary size due to pulling all io libraries - any thoughts? Can this be optional?

@dmateja
Copy link

dmateja commented Jan 17, 2019

There is a problem with 'std::string _what;' if you throw this exception from debug dll and try to catch in release exe (or vice versa) in MSVC - debug std::string != release std::string.
Is this case is important?

@beinhaerter
Copy link
Contributor Author

@annagrin Understood, good point. In my personal use case I don't mind because I am using iostream anyway (and I want to see the value!) but in a more general approach this matters. The question now for me is: how much interest exists in having a more verbose exception? If that is not of interest, there is no need to do more work on it here. If it is of interest, then let's talk about how I can improve it. Possible ways could be:

  1. only with stringstream, we can see the types and the value
  2. only with low level string functions, we can see the types but not the value
  3. don't change, it does not make much sense to see the types because a program often does the same narrowing in different places
  4. user can choose between 1 and 2 using a macro
    I also thought about other ways making stringstream optional, but I don't see a good way to do it and still comply with the one definition rule.

@dmateja Good point, but I am not sure if this is something that is supported/relevant. With regard to the one definition rule that would mean that you must have a library interface that is very restricted. But of course - GSL is a general purpose library and should not make unnecessary assumptions about the use cases. Maybe someone from the MS staff can share his/her sights?

@hsutter
Copy link
Collaborator

hsutter commented Oct 10, 2019

C++ Core Guidelines editors' call: This seems to be asking for a debugging aid that can be expensive in release mode. It could be added in debug mode only, and then only for projects that are not supposed to change their dependencies between debug and release modes (which seems undesirable because it limits use of GSL in such projects). Better would be to use the debugger to view the information if an exception is thrown, such as by setting a breakpoint in the implementation of narrow.

@beinhaerter
Copy link
Contributor Author

My debug output also helps when a narrow throws at a customer site. Setting a break point and inspecting the code in the debugger is not possible at a customer site.
But I understand the cost argument, so I'll close the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants