-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[clang][NFC] Refactor replaceExternalDecls to use llvm::any_of #143275
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
[clang][NFC] Refactor replaceExternalDecls to use llvm::any_of #143275
Conversation
|
@llvm/pr-subscribers-clang Author: Samarth Narang (snarang181) ChangesThis patch simplifies the declaration replacement logic in replaceExternalDecls by replacing a manual loop with an idiomatic use of llvm::any_of. This improves code readability and aligns with common LLVM coding style. Full diff: https://github.com/llvm/llvm-project/pull/143275.diff 1 Files Affected:
diff --git a/clang/include/clang/AST/DeclContextInternals.h b/clang/include/clang/AST/DeclContextInternals.h
index b17b7627ac90c..a1071f62d5fae 100644
--- a/clang/include/clang/AST/DeclContextInternals.h
+++ b/clang/include/clang/AST/DeclContextInternals.h
@@ -176,14 +176,10 @@ class StoredDeclsList {
DeclListNode::Decls *Tail = erase_if([Decls](NamedDecl *ND) {
if (ND->isFromASTFile())
return true;
- // FIXME: Can we get rid of this loop completely?
- for (NamedDecl *D : Decls)
- // Only replace the local declaration if the external declaration has
- // higher visibilities.
- if (D->getModuleOwnershipKind() <= ND->getModuleOwnershipKind() &&
- D->declarationReplaces(ND, /*IsKnownNewer=*/false))
- return true;
- return false;
+ return llvm::any_of(Decls, [ND](NamedDecl *D) {
+ return D->getModuleOwnershipKind() <= ND->getModuleOwnershipKind() &&
+ D->declarationReplaces(ND, /*IsKnownNewer=*/false);
+ });
});
// Don't have any pending external decls any more.
|
|
@ChuanqiXu9, requesting your review here. |
|
IIUC the |
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.
Please remain the comments.
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.
I added the comment back. Is that what you meant?
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.
Yes. We should keep both.
dc4e3c9 to
8446545
Compare
OK. In any case, the usage of |
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.
Here is another missing comment.
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.
OK. Should we keep the FIXME?
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.
Retain all now, I think.
9bee4e3 to
ff09e5c
Compare
ff09e5c to
f849821
Compare
…143275) This patch simplifies the declaration replacement logic in replaceExternalDecls by replacing a manual loop with an idiomatic use of llvm::any_of. This improves code readability and aligns with common LLVM coding style.
This patch simplifies the declaration replacement logic in replaceExternalDecls by replacing a manual loop with an idiomatic use of llvm::any_of. This improves code readability and aligns with common LLVM coding style.