sepavloff marked an inline comment as done.
sepavloff added inline comments.


================
Comment at: lib/CodeGen/CGExprConstant.cpp:1403
+  if (auto *IL = dyn_cast_or_null<InitListExpr>(Init)) {
+    if (InitTy->isConstantArrayType()) {
+      for (auto I : IL->inits())
----------------
rjmccall wrote:
> Do you actually care if it's an array initialization instead of a struct/enum 
> initialization?
If this code is enabled for for records too, some tests start to fail. For 
instance, the code:
```
union { int i; double f; } u2 = { };
```
produces output:
```
%union.anon = type { double }
@u2 = global %union.anon zeroinitializer, align 4
```
while previously it produced:
```
@u2 = global { i32, [4 x i8] } { i32 0, [4 x i8] undef }, align 4
```
The latter looks more correct.


================
Comment at: lib/CodeGen/CGExprConstant.cpp:1413
+  } else if (!Init->isEvaluatable(CE.CGM.getContext())) {
+    return false;
+  } else if (InitTy->hasPointerRepresentation()) {
----------------
rjmccall wrote:
> Aren't the null-pointer and integer-constant-expression checks below already 
> checking this?  Also, `isEvaluatable` actually computes the full value 
> internally (as an `APValue`), so if you're worried about the memory and 
> compile-time effects of producing such a value, you really shouldn't call it.
> 
> You could reasonably move this entire function to be a method on `Expr` that 
> takes an `ASTContext`.
Comment for `EvaluateAsRValue` says that it tries calculate expression 
agressively. Indeed, for the code:
```
  decltype(nullptr) null();
  int *p = null();
```
compiler ignores potential side effect of `null()` and removes the call, 
leaving only zero initialization. `isNullPointerConstant` behaves similarly.


================
Comment at: lib/CodeGen/CGExprConstant.cpp:1417
+    if (Init->EvaluateAsRValue(ResVal, CE.CGM.getContext()))
+      return ResVal.Val.isLValue() && ResVal.Val.isNullPointer();
+  } else {
----------------
rjmccall wrote:
> There's a `isNullPointerConstant` method (you should use 
> `NPC_NeverValueDependent`).
It make code more readable. Thank you!


Repository:
  rC Clang

https://reviews.llvm.org/D46241



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to