[cfe-commits] r67059 - in /cfe/trunk: include/clang/AST/ include/clang/Basic/ lib/AST/ lib/CodeGen/ lib/Sema/ test/Parser/ test/SemaCXX/
Douglas Gregor
dgregor at apple.com
Mon Mar 16 17:02:57 PDT 2009
On Mar 16, 2009, at 4:22 PM, Sebastian Redl wrote:
> Author: cornedbee
> Date: Mon Mar 16 18:22:08 2009
> New Revision: 67059
>
> URL: http://llvm.org/viewvc/llvm-project?rev=67059&view=rev
> Log:
> Almost complete implementation of rvalue references. One bug, and a
> few unclear areas. Maybe Doug can shed some light on some of the
> fixmes.
An excellent trade, indeed!
I'll do a detailed review tomorrow, but here are some comments on the
FIXMEs.
> @@ -2641,9 +2680,12 @@
> // C++ [expr]: If an expression initially has the type "reference
> to T", the
> // type is adjusted to "T" prior to any further analysis, the
> expression
> // designates the object or function denoted by the reference, and
> the
> - // expression is an lvalue.
> + // expression is an lvalue unless the reference is an rvalue
> reference and
> + // the expression is a function call (possibly inside parentheses).
> // FIXME: C++ shouldn't be going through here! The rules are
> different
> // enough that they should be handled separately.
> + // FIXME: Merging of lvalue and rvalue references is incorrect. C+
> + *really*
> + // shouldn't be going through here!
I wonder if C++ is actually going through here?
> if (const ReferenceType *RT = LHS->getAsReferenceType())
> LHS = RT->getPointeeType();
> if (const ReferenceType *RT = RHS->getAsReferenceType())
> @@ -2746,7 +2788,8 @@
> assert(false && "Non-canonical and dependent types shouldn't get
> here");
> return QualType();
>
> - case Type::Reference:
> + case Type::LValueReference:
> + case Type::RValueReference:
> case Type::MemberPointer:
> assert(false && "C++ should never be in mergeTypes");
> return QualType();
>
> Modified: cfe/trunk/lib/AST/Builtins.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Builtins.cpp?rev=67059&r1=67058&r2=67059&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/lib/AST/Builtins.cpp (original)
> +++ cfe/trunk/lib/AST/Builtins.cpp Mon Mar 16 18:22:08 2009
> @@ -183,7 +183,7 @@
> if (Type->isArrayType()) {
> Type = Context.getArrayDecayedType(Type);
> } else {
> - Type = Context.getReferenceType(Type);
> + Type = Context.getLValueReferenceType(Type);
> }
> break;
> case 'V': {
> @@ -224,8 +224,9 @@
> Type = Context.getPointerType(Type);
> break;
> case '&':
> - Type = Context.getReferenceType(Type);
> + Type = Context.getLValueReferenceType(Type);
> break;
> + // FIXME: There's no way to have a built-in with an rvalue
> ref arg.
Right, and that's a good thing (IMHO). I'd just remove this FIXME.
> Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=67059&r1=67058&r2=67059&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> +++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Mar 16 18:22:08 2009
> @@ -696,6 +696,7 @@
> /// as GCC.
> static int EvaluateBuiltinClassifyType(const CallExpr *E) {
> // The following enum mimics the values returned by GCC.
> + // FIXME: Does GCC differ between lvalue and rvalue references
> here?
> enum gcc_type_class {
> no_type_class = -1,
> void_type_class, integer_type_class, char_type_class,
Oh, interesting. I don't recall this being part of the rvalue-
references implementation that went into GCC, so I suspect GCC does
not differentiate. We can check it with GCC >= 4.3.
> // Compute some basic properties of the types and the initializer.
> + bool isRValRef = DeclType->isRValueReferenceType();
> bool DerivedToBase = false;
> Expr::isLvalueResult InitLvalue = Init->isLvalue(Context);
> ReferenceCompareResult RefRelationship
> @@ -1738,6 +1740,15 @@
> RefRelationship >= Ref_Compatible_With_Added_Qualification) {
> BindsDirectly = true;
>
> + // Rvalue references cannot bind to lvalues (N2812).
> + // FIXME: This part of rvalue references is still in flux.
> Revisit later.
> + if (isRValRef) {
> + if (!ICS)
> + Diag(Init->getSourceRange().getBegin(),
> diag::err_lvalue_to_rvalue_ref)
> + << Init->getSourceRange();
> + return true;
> + }
> +
This isn't in flux any more; earlier this month, in Summit, NJ, the C+
+ committee accepted the proposed wording that bans binding rvalue
references to lvalues. The final document is N2844, which will be in
the post-Summit mailing (available in ~2 weeks); I've attached a copy
of N2844.
> if (ICS) {
> // C++ [over.ics.ref]p1:
> // When a parameter of reference type binds directly (8.5.3)
> @@ -1774,7 +1785,9 @@
> // 92) (this conversion is selected by enumerating the
> // applicable conversion functions (13.3.1.6) and choosing
> // the best one through overload resolution (13.3)),
> - if (!SuppressUserConversions && T2->isRecordType()) {
> + // FIXME: Without standard language for N2812, the rvalue
> reference treatment
> + // here is pretty much a guess.
> + if (!isRValRef && !SuppressUserConversions && T2->isRecordType()) {
It looks like you guessed correctly, but I'll think about it again
when I get to a do a more thorough review.
> + // FIXME: C++0x [except.handle] names the handler as cv T or cv
> T&, i.e.
> + // rvalue references aren't there. Oversight or intentional?
Intentional. N2844 explicitly bans the use of rvalue references in the
handler.
> // FIXME: Need to test for ability to copy-construct and destroy the
> // exception variable.
> // FIXME: Need to check for abstract classes.
>
> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=67059&r1=67058&r2=67059&view=diff
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Mon Mar 16 18:22:08 2009
> @@ -764,7 +764,7 @@
> // the constructor or conversion operator, and then cope with the
> // standard conversions.
> ImpCastExprToType(From, ToType.getNonReferenceType(),
> - ToType->isReferenceType());
> + ToType->isLValueReferenceType());
> return false;
>
> case ImplicitConversionSequence::EllipsisConversion:
> @@ -800,7 +800,7 @@
> // FIXME: Create a temporary object by calling the copy
> // constructor.
> ImpCastExprToType(From, ToType.getNonReferenceType(),
> - ToType->isReferenceType());
> + ToType->isLValueReferenceType());
> return false;
> }
>
> @@ -893,8 +893,10 @@
> break;
>
> case ICK_Qualification:
> + // FIXME: Not sure about lvalue vs rvalue here in the presence of
> + // rvalue references.
> ImpCastExprToType(From, ToType.getNonReferenceType(),
> - ToType->isReferenceType());
> + ToType->isLValueReferenceType());
> break;
This looks right to me. If we're implicitly converting to an rvalue-
reference type, then the implicit conversion produces an rvalue.
>
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- cfe/trunk/lib/Sema/SemaType.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaType.cpp Mon Mar 16 18:22:08 2009
> @@ -319,8 +319,16 @@
> ///
> /// \returns A suitable reference type, if there are no
> /// errors. Otherwise, returns a NULL type.
> -QualType Sema::BuildReferenceType(QualType T, unsigned Quals,
> +QualType Sema::BuildReferenceType(QualType T, bool LValueRef,
> unsigned Quals,
> SourceLocation Loc,
> DeclarationName Entity) {
> + if (LValueRef) {
> + if (const RValueReferenceType *R = T->getAsRValueReferenceType
> ()) {
> + // FIXME: Find the C++0x reference for reference collapsing.
> + // In reference collapsing, lvalue refs win over rvalue refs.
> + return Context.getLValueReferenceType(R->getPointeeType()).
> + getQualifiedType(Quals);
> + }
> + }
IIRC, in the reference-collapsing case we drop the qualifiers applied
to the outer reference.
Thanks for working on this! Thorough review to follow, tomorrow.
- Doug
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20090316/4104d823/attachment.html>
More information about the cfe-commits
mailing list