Skip to content

[TBDGen] Side-effects and asserts don't mix. #8446

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

Merged
merged 1 commit into from
Mar 31, 2017

Conversation

huonw
Copy link
Contributor

@huonw huonw commented Mar 30, 2017

Fixes rdar://problem/31343022.

@huonw
Copy link
Contributor Author

huonw commented Mar 30, 2017

@swift-ci Please smoke test and merge.

@gottesmm
Copy link
Contributor

@huonw if your smoke test was a python failure, I think @erg fixed it. I am going to restart it for you.

@gottesmm
Copy link
Contributor

@swift-ci Please smoke test os x platform

@@ -37,7 +37,8 @@ class TBDGenVisitor : public ASTVisitor<TBDGenVisitor> {
StringSet &Symbols;

void addSymbol(StringRef name) {
assert(Symbols.insert(name).second && "already inserted");
auto inserted = Symbols.insert(name).second;
assert(inserted && "already inserted");
Copy link
Contributor

Choose a reason for hiding this comment

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

To avoid a warning with asserts off, add a (void) inserted; also

Fixes rdar://problem/31343022.
@huonw huonw force-pushed the no-side-effects-in-asserts branch from 61a2b30 to d710b46 Compare March 31, 2017 00:21
@huonw
Copy link
Contributor Author

huonw commented Mar 31, 2017

@swift-ci Please smoke test and merge.

4 similar comments
@huonw
Copy link
Contributor Author

huonw commented Mar 31, 2017

@swift-ci Please smoke test and merge.

@huonw
Copy link
Contributor Author

huonw commented Mar 31, 2017

@swift-ci Please smoke test and merge.

@gottesmm
Copy link
Contributor

@swift-ci Please smoke test and merge.

@gottesmm
Copy link
Contributor

@swift-ci Please smoke test and merge.

@huonw huonw merged commit a2644a5 into swiftlang:master Mar 31, 2017
@huonw huonw deleted the no-side-effects-in-asserts branch March 31, 2017 16:53
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.

3 participants