-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Indexing] Don't verify mangling of USRs #81941
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
Conversation
3d628b3
to
ca2060f
Compare
@swift-ci Please smoke test |
Seems like a bug with the Observation module's dependencies (cc: @phausler) |
lib/AST/ASTMangler.cpp
Outdated
verify(Storage.str(), Flavor); | ||
return finalize(); | ||
} | ||
|
||
std::string ASTMangler::mangleDeclAsUSR(const ValueDecl *Decl, | ||
StringRef USRPrefix) { | ||
return (llvm::Twine(USRPrefix) + mangleAnyDecl(Decl, false)).str(); | ||
#ifndef NDEBUG |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of NDEBUG can you check if (CONDITIONAL_ASSERT_enabled())
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea. Thanks, Slava.
@swift-ci please test Windows platform |
ca2060f
to
ef95fda
Compare
@swift-ci Please smoke test |
Verifying USR mangling adds ~30% overhead to indexing times. Since an incorrect USR mangling doesn't result in a correctness issue at the same level as a miscompile, save those 30% in non-assert builds.
ef95fda
to
563f971
Compare
@swift-ci Please smoke test |
@swift-ci Please smoke test macOS |
Verifying USR mangling adds ~30% overhead to indexing times. Since an incorrect USR mangling doesn't result in a correctness issue at the same level as a miscompile, save those 30% in non-assert builds.