[cfe-commits] r57935 - in /cfe/trunk: include/clang/AST/Type.h lib/AST/Expr.cpp lib/Sema/Sema.h lib/Sema/SemaExpr.cpp lib/Sema/SemaOverload.cpp lib/Sema/SemaType.cpp test/SemaCXX/decl-expr-ambiguity.cpp test/SemaCXX/qualification-conversion.cpp

[Doug Gregor]

On Tue, Oct 21, 2008 at 8:33 PM, Chris Lattner <clattner at apple.com> wrote:
> On Oct 21, 2008, at 4:43 PM, Douglas Gregor wrote:
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=57935&view=rev
>> Log:
>> Initial step toward supporting qualification conversions (C++ 4.4).
>> +  /// isMoreQualifiedThan - Determine whether this type is more
>> +  /// qualified than the Other type. For example, "const volatile int"
>> +  /// is more qualified than "const int", "volatile int", and
>> +  /// "int". However, it is not more qualified than "const volatile
>> +  /// int".
>> +  bool isMoreQualifiedThan(QualType Other) const {
>> +    unsigned MyQuals = this->getCVRQualifiers();
>> +    unsigned OtherQuals = Other.getCVRQualifiers();
>> +    return MyQuals != OtherQuals && (MyQuals | OtherQuals) == MyQuals;
>> +  }
>
> You probably don't care about Addr space qualifiers, and it is fine to defer
> the issue... but it would probably be best to assert that
> "Other.getAddressSpace() == 0" (and 'this') until we handle them.  Likewise
> in isAtLeastAsQualifiedAs.

We're now using isAtLeastAsQualifiedAs in
Sema::CheckPointerTypesForAssignment. That currently ignores
address-space qualifiers, but adding those asserts would turn code
that would now compile into something that crashes the compiler.

>>
>> +  /// FIXME: Expressions can't have reference type, so the following
>> +  /// isn't needed.
>>  if (TR->isReferenceType()) // C++ [expr]
>>    return LV_Valid;
>
> I think a lot of the reference logic in the compiler is currently busted.
>  The only real case we support references today is for some vaarg builtins
> IIRC.  Does anything in the regression suite fail if you remove this today?
>  If not, I'd just rip it (and any other bogosity) out.

I'm going to fix all of these at once in a later batch.

>>
>>
>> @@ -356,7 +360,11 @@
>>    return LV_Valid;
>>  case DeclRefExprClass: { // C99 6.5.1p2
>>    const Decl *RefdDecl = cast<DeclRefExpr>(this)->getDecl();
>> -    if (isa<VarDecl>(RefdDecl) || isa<ImplicitParamDecl>(RefdDecl))
>> +    if (isa<VarDecl>(RefdDecl) ||
>> +        isa<ImplicitParamDecl>(RefdDecl) ||
>> +        // C++ 3.10p2: An lvalue refers to an object or function.
>> +        isa<FunctionDecl>(RefdDecl) ||
>> +        isa<OverloadedFunctionDecl>(RefdDecl))
>
> Does this make sense to be a predicate on valuedecl?

I don't think it's necessary unless we need this kind of check again later.

>> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Oct 21 18:43:52 2008
>> @@ -450,7 +450,7 @@
>>  }
>>  // If this reference is not in a block or if the referenced variable is
>>  // within the block, create a normal DeclRefExpr.
>> -  return new DeclRefExpr(VD, VD->getType(), Loc);
>> +  return new DeclRefExpr(VD, GetNonReferenceType(VD->getType()), Loc);
>
> Very minor and pedantic, but this is a hot code path for both C and C++, and
> this adds a new out-of-line call.  How about something like:
>
> QualType ValTy = VD->getType();
> if (ValTy->isReferenceType()) ValTy = GetNonReferenceType(ValTy);
> return new DeclRefExpr(VD, ValTy, Loc);

Why not just make GetNonReferenceType inline? It's small enough, and
it's going to happy in lots of hot code paths.

>> +/// GetNonReferenceType - If Type is a reference type (e.g., const
>> +/// int&), returns the type that the reference refers to ("const
>> +/// int"). Otherwise, returns the type itself. This routine is used
>> +/// throughout to implement C++ 5p6:
>> +///
>> +///   If an expression initially has the type "reference to T" (8.3.2,
>> +///   8.5.3), 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.
>> +QualType Sema::GetNonReferenceType(QualType Type)
>> +{
>> +  if (const ReferenceType *RefType = Type->getAsReferenceType())
>> +    return RefType->getPointeeType();
>> +  else
>> +    return Type;
>> +}
>
> This seems more natural to be a method on QualType, what do you think?

Fine by me. I didn't put it there previously because it's something
that's only really there as a shorthand for use by Sema.

  - Doug