-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[clang][NFC] Simplify implementation of IgnoreExprNodes
#164193
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
|
@llvm/pr-subscribers-clang Author: Victor Chernyakin (localspook) ChangesUsing a fold instead of template recursion. Full diff: https://github.com/llvm/llvm-project/pull/164193.diff 1 Files Affected:
diff --git a/clang/include/clang/AST/IgnoreExpr.h b/clang/include/clang/AST/IgnoreExpr.h
index 917bada61fa6f..bf098f3f09068 100644
--- a/clang/include/clang/AST/IgnoreExpr.h
+++ b/clang/include/clang/AST/IgnoreExpr.h
@@ -17,16 +17,6 @@
#include "clang/AST/ExprCXX.h"
namespace clang {
-namespace detail {
-/// Given an expression E and functions Fn_1,...,Fn_n : Expr * -> Expr *,
-/// Return Fn_n(...(Fn_1(E)))
-inline Expr *IgnoreExprNodesImpl(Expr *E) { return E; }
-template <typename FnTy, typename... FnTys>
-Expr *IgnoreExprNodesImpl(Expr *E, FnTy &&Fn, FnTys &&... Fns) {
- return IgnoreExprNodesImpl(std::forward<FnTy>(Fn)(E),
- std::forward<FnTys>(Fns)...);
-}
-} // namespace detail
/// Given an expression E and functions Fn_1,...,Fn_n : Expr * -> Expr *,
/// Recursively apply each of the functions to E until reaching a fixed point.
@@ -35,7 +25,7 @@ template <typename... FnTys> Expr *IgnoreExprNodes(Expr *E, FnTys &&... Fns) {
Expr *LastE = nullptr;
while (E != LastE) {
LastE = E;
- E = detail::IgnoreExprNodesImpl(E, std::forward<FnTys>(Fns)...);
+ ((E = std::forward<FnTys>(Fns)(E)), ...);
}
return E;
}
|
cor3ntin
left a 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.
Do we ever have more that 2 functions?
It might be simpler/faster to compile to just have
const Expr *IgnoreExprNodes(const Expr *E, const Expr*(*F)(const Expr*));
const Expr *IgnoreExprNodes(const Expr *E, const Expr*(*F1)(const Expr*),
const Expr*(*F2)(const Expr*));
|
We have this :D llvm-project/clang/lib/AST/Expr.cpp Lines 3203 to 3206 in 4d6f43d
|
We could use an array of functions for that (and apply the array in reverse) |
|
That would work, but I imagine it would be harder for the compiler to inline. I'm merging for now, but that could always be done in another PR |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/23312 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/202/builds/4500 Here is the relevant piece of the build log for the reference |
Using a fold instead of template recursion.
Using a fold instead of template recursion.