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