rsmith added inline comments.

================
Comment at: lib/AST/ExprConstant.cpp:5061
+          APValue RVal;
+          // FIXME: We need to make sure we're passing the right type that
+          // maintains cv-qualifiers.
----------------
faisalv wrote:
> I don't think we need this fixme - the type of the expression should be 
> correct - all other const checks for mutability have already been performed, 
> right?
When using `handleLValueToRValueConversion` to obtain the lvalue denoted by a 
reference, the type you pass should be the reference type itself 
(`FD->getType()`). This approach will give the wrong answer when using a 
captured volatile object:
```
void f() {
  volatile int n;
  constexpr volatile int *p = [&]{ return &n; }(); // should be accepted
}
```


================
Comment at: lib/AST/ExprConstant.cpp:5066-5068
+          assert(RVal.isLValue() && "Reference captures through their "
+                                    "corresponding field members must refer to 
"
+                                    "lvalues (VarDecls or FieldDecls)");
----------------
I don't see why this assert is correct. If we initialize a reference with a 
(constant-folded) dereferenced integer cast to pointer type, the value will 
have integer representation. Just remove the assert?


================
Comment at: lib/AST/ExprConstant.cpp:5473-5474
+      
+      if (HandleLValueMember(Info, E, Result,
+                             Info.CurrentCall->LambdaThisCaptureField)) {
+        if (Info.CurrentCall->LambdaThisCaptureField->getType()
----------------
Please use early-exit style (`if (!HandleLValueMember(...)) return false;`) 
here to reduce indentation and make it clearer that you only return false if a 
diagnostic has already been produced.


================
Comment at: lib/AST/ExprConstant.cpp:6338-6339
+    // occurred.
+    if (!CurFieldInit)
+      return false;
+
----------------
Returning `false` without producing a diagnostic (for the VLA case) is not OK. 
You should at least produce the basic "not a constant expression" note here.


Repository:
  rL LLVM

https://reviews.llvm.org/D29748



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

Reply via email to