RequirementMachine: Convert to new assertions #74631
Conversation
slavapestov
force-pushed
the
rqm-assertions
branch
2 times, most recently
from
d558624 to
db7fa8c
Compare
slavapestov
force-pushed
the
rqm-assertions
branch
2 times, most recently
from
21ea00b to
fcc0702
Compare
| @@ -43,7 +43,7 @@ | |||
|
|
|||
| #define ASSERT(expr) \ | |||
| do { \ | |||
| if (ASSERT_UNLIKELY(!expr)) { \ | |||
| if (ASSERT_UNLIKELY(!(expr))) { \ | |||
| ASSERT(!props->isConcreteType() && | ||
| "Concrete types do not have conformance access paths"); | ||
| auto conformsTo = props->getConformsTo(); | ||
| CONDITIONAL_ASSERT(std::find(conformsTo.begin(), conformsTo.end(), protocol) && |
There was a problem hiding this comment.
Not much point to having CONDITIONAL_ASSERT inside a CONDITIONAL_ASSERT_enabled block.
There was a problem hiding this comment.
Ah, good catch.
| auto prefixBegin = lookupTerm.begin(); | ||
| auto prefixEnd = lookupTerm.end() - Key.size(); | ||
| assert(std::equal(prefixEnd, lookupTerm.end(), Key.begin()) && | ||
| "This is not the bag you're looking for"); | ||
| DEBUG_ASSERT(std::equal(prefixEnd, lookupTerm.end(), Key.begin()) && |
There was a problem hiding this comment.
Why DEBUG_ASSERT here? (Genuinely curious.)
There was a problem hiding this comment.
It seemed that in a few places the asserts are extremely defensive because the caller establishes the condition immediately... but maybe that line of reasoning defeats the purpose of asserts!
I also used DEBUG_ASSERT in a few 'inner loops'. I was going to profile this patch anyway so I can play around with things a bit to see where we stand.
There was a problem hiding this comment.
Thanks for that clarification. It helps to hear people talk about how they think of these things.
|
I'm curious about the use of |
slavapestov
force-pushed
the
rqm-assertions
branch
from
ae63615 to
4795a13
Compare
slavapestov
force-pushed
the
rqm-assertions
branch
from
af03db2 to
273c4b2
Compare
|
@swift-ci Please smoke test |
No description provided.