Skip to content

Conversation

kroening
Copy link
Collaborator

This adds a class for exceptions in the Verilog preprocessor, as opposed to throwing the integer 0.

Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

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

Approving, but I'd appreciate adjusting the interface as suggested below.

error().source_location = source_location;
error() << "unknown preprocessor directive \"" << text << "\"" << eom;
throw 0;
throw verilog_preprocessor_errort() << "unknown preprocessor directive \"" << text << "\"";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd have a preference towards RAII: could the constructor please require (!) a string to be passed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The class meets the RAII criteria; in particular, all invariants are satisfied. It's like a container, and as such may be constructed empty.

This adds a class for exceptions in the Verilog preprocessor, as opposed to
throwing the integer 0.
@kroening kroening force-pushed the verilog_preprocessor_error branch from a3d783f to b581a03 Compare September 23, 2023 23:49
Comment on lines +607 to +608
throw verilog_preprocessor_errort()
<< "unknown preprocessor directive \"" << text << "\"";
Copy link
Collaborator

Choose a reason for hiding this comment

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

I accept that this satisfies RAII, but I am still wondering whether a constructor that takes a std::string wouldn't be the better way forward.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It would be a bit weird, e.g., in this case you would have
throw verilog_preprocessor_errort("unknown preprocessor directive \"") << text << "\"";

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

"Looks" wrong.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I would have used + instead of << and have passed the entire string to the constructor. But it’s just a matter of taste, no big deal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, but then you'll want numbers, tokens (which have a << operator, but no +), etc.

@kroening kroening merged commit fac56d9 into main Sep 25, 2023
@kroening kroening deleted the verilog_preprocessor_error branch September 25, 2023 15:43
Romy15200 pushed a commit to Romy15200/nws that referenced this pull request Aug 19, 2025
Verilog: add a class for exceptions in the preprocessor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants