yronglin marked 4 inline comments as done.
yronglin added a comment.
@cor3ntin Thanks for your review!
================
Comment at: clang/include/clang/Sema/Sema.h:1357
+ /// Whether rewrite the default argument.
+ bool IsRewriteDefaultArgument = false;
+
----------------
cor3ntin wrote:
> Can `IsRewriteDefaultArgument` and `MaterializePRValueInDiscardStatement`
> have different values?
> Maybe `MaterializePRValueInDiscardStatement` is sufficient
Currently they have the same value, but they mean different things, so I use
two separated variables.
================
Comment at: clang/include/clang/Sema/Sema.h:1361
+ /// Eg. Extending the lifetime of temporaries in the init expression.
+ bool IsCheckingCXXForRangeInitVariable = false;
+
----------------
cor3ntin wrote:
> Either `IsInLifetimeExtendingContext` or `IsInCXXForRangeInitializer` seem
> like better names
Thanks for your suggestion, `IsInLifetimeExtendingContext ` looks good to me, I
can use this var name later.
================
Comment at: clang/lib/Sema/SemaExpr.cpp:6250
+ CallLoc, Param, CurContext};
EnsureImmediateInvocationInDefaultArgs Immediate(*this);
ExprResult Res;
----------------
@cor3ntin Does `EnsureImmediateInvocationInDefaultArgs` need to be renamed to a
more generic name?
================
Comment at: clang/lib/Sema/SemaExpr.cpp:18168
Prev.InImmediateEscalatingFunctionContext;
-
Cleanup.reset();
----------------
cor3ntin wrote:
> Whitespace only change
Thanks, I'll remove this.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8206-8209
+ // We do not materialize temporay by default in order to avoid creating
+ // unnecessary temporary objects. If we skip this step, IR generation is
+ // able to synthesize the storage for itself in the aggregate case, and
+ // adding the extra node to the AST is just clutter.
----------------
cor3ntin wrote:
> Correct me if I'm wrong but the thing we want to avoid here is to avoid
> creating unnecessary AST nodes, right?
Yeah, ignore unnecessary AST nodes and runtime memory allocation. The redundant
alloca/store instructions may be removed during the optimization phase.
Eg.
```
static_cast<void>(42);
```
```
define noundef i32 @main() {
entry:
%retval = alloca i32, align 4
%ref.tmp = alloca i32, align 4
store i32 0, ptr %retval, align 4
store i32 42, ptr %ref.tmp, align 4
call void @f()()
ret i32 0
}
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D153701/new/
https://reviews.llvm.org/D153701
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
