Squelch some dead code warnings. #13186
Conversation
|
@swift-ci Please smoke test |
lib/FrontendTool/FrontendTool.cpp
Outdated
| @@ -393,7 +393,7 @@ LLVM_ATTRIBUTE_NOINLINE | |||
| static void debugFailWithAssertion() { | |||
| // This assertion should always fail, per the user's request, and should | |||
| // not be converted to llvm_unreachable. | |||
| assert(0 && "This is an assertion!"); | |||
| assert((0) && "This is an assertion!"); | |||
There was a problem hiding this comment.
llvm_unreachable is compiled in the same cases as asserts, so there's no point in not making this llvm_unreachable
There was a problem hiding this comment.
Presumably there's a reason why this specifically says it should not be llvm_unreachable.
There was a problem hiding this comment.
Yeah, a misunderstanding on the author's part :)
There was a problem hiding this comment.
Well, the original patch did this:
commit 9df53c5c71b5bb9252ee5d13cd349295a6d19602
Author: Connor Wakamo <[email protected]>
Date: Fri Feb 7 22:30:40 2014 +0000
[frontend] Switch -debug-assert-* from llvm_unreachable() to assert(0).
Swift SVN r13656
There was a problem hiding this comment.
The flag is only used in one test, test/Driver/assert.swift. If the test passes with it changed to an llvm_unreachable I would keep it that way.
There was a problem hiding this comment.
llvm_unreachable actually has an effect in no-asserts build (which is why I almost never introduce it in places where someone previously had an assert, and almost certainly why it was not used above).
It becomes __builtin_unreachable() which the optimizer takes to mean "party on!", which in practice for LLVM I believe means it deletes all code following this point until the next join in control flow (although I could be wrong - it does not seem to turn into ud2 on x86, but it definitely deletes useful code after that point, resulting in unpredictable crashes happening later).
[edited a few times to conditionalize statements :)]
lib/Parse/ParseType.cpp
Outdated
| @@ -184,11 +184,11 @@ ParserResult<TypeRepr> Parser::parseTypeSimple(Diag<> MessageID, | |||
| TypeSpecifier = VarDecl::Specifier::InOut; | |||
| } else if (Tok.is(tok::identifier)) { | |||
| if (Tok.getRawText().equals("__shared")) { | |||
| assert(false); | |||
| assert((false)); | |||
There was a problem hiding this comment.
This and all the others should be llvm_unreachable
There was a problem hiding this comment.
Yes, true. Blindly trying to kill off warnings without paying enough attention. Thanks for noticing.
There was a problem hiding this comment.
Ah sorry, I removed this part in #13173
Fix up some dead code warnings as well as a warning for testing whether an unsigned number is non-negative (which it always is).
|
@swift-ci Please smoke test |
No description provided.