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

V3Ast.cpp gives warning with -Winvalid-noreturn #1440

Closed
veripoolbot opened this issue May 14, 2019 · 3 comments
Closed

V3Ast.cpp gives warning with -Winvalid-noreturn #1440

veripoolbot opened this issue May 14, 2019 · 3 comments

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented May 14, 2019


Author Name: Kevin Kiningham (@kkiningh)
Original Redmine Issue: 1440 from https://www.veripool.org


Compiling src/V3Ast.cpp produces the following warning when compiled with -Winvalid-noreturn (Apple LLVM version 10.0.0)

src/V3Ast.cpp:1070:92: warning: function declared 'noreturn' should not return [-Winvalid-noreturn]                                                                                                             
void AstNode::v3errorEndFatal(std::ostringstream& str) const { v3errorEnd(str); assert(0); }
                                                                                            ^

This is fixed pretty easily by adding VL_UNREACHABLE after the assert (patch attached). I'm not 100% sure that's the correct solution though, since assert may return if compiled with NDEBUG defined. Would it be better to turn the assert(0) into an abort()?

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 15, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-05-15T00:51:34Z


Your fix looks good, there were also two other similar sections of code I patched.

The assert will never get hit, as v3errorEnd exits before then. The assert is there to trick formal tools (that ignore the GNU attributes) into knowing the code is non-return.

Note Apple's LLVM 10.0.0 seems to have stuff not in the most recent clang++ 8.0 so there might be additional issues.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 15, 2019


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2019-05-15T00:52:07Z


Pushed to git towards 4.016.

@veripoolbot

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented May 15, 2019


Original Redmine Comment
Author Name: Kevin Kiningham (@kkiningh)
Original Date: 2019-05-15T00:52:55Z


Awesome, thanks for the clarification!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.