adds an unimplemented exception and exception catching in drivers #555
Conversation
lib/exceptions.h
Outdated
| @@ -48,13 +48,31 @@ class CompilerBug final : public P4CExceptionBase { | |||
| template <typename... T> | |||
| CompilerBug(const char* format, T... args) | |||
| : P4CExceptionBase(format, args...) | |||
| { message = "COMPILER BUG:\n" + message; } | |||
| // \e[31m prints the text in red | |||
| { message = "\e[31mCOMPILER BUG\e[0m:\n" + message; } | |||
There was a problem hiding this comment.
No "\e[5m"? Kidding. Should we remove the all-caps?
There was a problem hiding this comment.
Not sure "\e5m" will convey the right message: can it blink slow enough to wait for someone to fix the bug? :)
Yeah, camel case will make it less threatening.
There was a problem hiding this comment.
Is this portable, or will it look terrible in non-xterm terminals?
I usually have my shell in emacs.
Adds a new type of P4Exception: CompilerUnimplemented to replace instances where we use BUG/BUG_CHECK as a placeholder for features that we have not yet implemented. Also adds exception catching in the compiler drivers, such that the compiler exits cleanly.
| P4::FrontEnd frontend; | ||
| frontend.addDebugHook(hook); | ||
| program = frontend.run(options, program); | ||
| } catch (const Util::P4CExceptionBase &bug) { |
There was a problem hiding this comment.
There is a benefit for not catching exceptions: makes debugging easier.
Otherwise you have to set breakpoints manually in all these places.
There was a problem hiding this comment.
In theory for debugging purposes you could use catch throw in gdb. I haven't ever actually tried that, though, so I'm not sure how well it works in practice.
lib/exceptions.h
Outdated
| @@ -48,13 +48,31 @@ class CompilerBug final : public P4CExceptionBase { | |||
| template <typename... T> | |||
| CompilerBug(const char* format, T... args) | |||
| : P4CExceptionBase(format, args...) | |||
| { message = "COMPILER BUG:\n" + message; } | |||
| // \e[31m prints the text in red | |||
| { message = "\e[31mCOMPILER BUG\e[0m:\n" + message; } | |||
There was a problem hiding this comment.
Is this portable, or will it look terrible in non-xterm terminals?
I usually have my shell in emacs.
lib/exceptions.h
Outdated
| }; | ||
|
|
||
| // This class indicates an unimplemented feature in the compiler | ||
| class CompilerUnimplemented final : public P4CExceptionBase { |
There was a problem hiding this comment.
You probably want a JavaDoc comment.
Also, this is not yet used anywhere, perhaps you want at least one use case.
There was a problem hiding this comment.
We will start using it during code reviews. In particular, we are working on the bmv2 backend and there are a few places there. Coming ...
There was a problem hiding this comment.
It is definitely portable. Works even on mobile phones. Emacs shell as well:
And address reviewer's comments + prettify.
…lang#555) * adds an unimplemented exception and exception catching in drivers Adds a new type of P4Exception: CompilerUnimplemented to replace instances where we use BUG/BUG_CHECK as a placeholder for features that we have not yet implemented. Also adds exception catching in the compiler drivers, such that the compiler exits cleanly. * fix unit test to account for escape sequences And address reviewer's comments + prettify.
Adds a new type of P4Exception: CompilerUnimplemented to replace
instances where we use BUG/BUG_CHECK as a placeholder for features
that we have not yet implemented.
Also adds exception catching in the compiler drivers, such that the
compiler exits cleanly.