r284685 - Refactor and simplify Sema::FindCompositePointerType. No functionality change intended.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 20 01:02:44 PDT 2016


Should hopefully be fixed by r284701.

On Thu, Oct 20, 2016 at 12:55 AM, Richard Smith <richard at metafoo.co.uk>
wrote:

> I think I understand the MSVC bug here; workaround incoming.
>
> On Wed, Oct 19, 2016 at 11:54 PM, Mike Aizatsky <aizatsky at google.com>
> wrote:
>
>> I think this breaks windows bot:
>>
>> http://lab.llvm.org:8011/builders/sanitizer-windows/builds/
>> 30745/steps/build%20clang%20lld/logs/stdio
>>
>> C:\PROGRA~2\MICROS~1.0\VC\bin\AMD64_~2\cl.exe   /nologo /TP -DCLANG_ENABLE_ARCMT -DCLANG_ENABLE_OBJC_REWRITER -DCLANG_ENABLE_STATIC_ANALYZER -DGTEST_HAS_RTTI=0 -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_GNU_SOURCE -D_HAS_EXCEPTIONS=0 -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools\clang\lib\Sema -IC:\b\slave\sanitizer-windows\llvm\tools\clang\lib\Sema -IC:\b\slave\sanitizer-windows\llvm\tools\clang\include -Itools\clang\include -Iinclude -IC:\b\slave\sanitizer-windows\llvm\include /DWIN32 /D_WINDOWS   /W4 -wd4141 -wd4146 -wd4180 -wd4244 -wd4258 -wd4267 -wd4291 -wd4345 -wd4351 -wd4355 -wd4456 -wd4457 -wd4458 -wd4459 -wd4503 -wd4624 -wd4722 -wd4800 -wd4100 -wd4127 -wd4512 -wd4505 -wd4610 -wd4510 -wd4702 -wd4245 -wd4706 -wd4310 -wd4701 -wd4703 -wd4389 -wd4611 -wd4805 -wd4204 -wd4577 -wd4091 -wd4592 -wd4319 -wd4324 -w14062 -we4238 /Zc:inline /Zc:strictStrings /Oi /Zc:rvalueCast /Zc:sizedDealloc- /MD /O2 /Ob2   -UNDEBUG  /EHs-c- /GR- /showIncludes /Fotools\clang\lib\Sema\CMakeFiles\clangSema.dir\SemaExprCXX.cpp.obj /Fdtools\clang\lib\Sema\CMakeFiles\clangSema.dir\ /FS -c C:\b\slave\sanitizer-windows\llvm\tools\clang\lib\Sema\SemaExprCXX.cpp
>> C:\b\slave\sanitizer-windows\llvm\tools\clang\lib\Sema\SemaExprCXX.cpp(5702): error C2326: 'clang::Sema::FindCompositePointerType::Conversion::Conversion(clang::Sema &,clang::SourceLocation,clang::Expr *&,clang::Expr *&,clang::QualType)': function cannot access 'Composite'
>> C:\b\slave\sanitizer-windows\llvm\tools\clang\lib\Sema\SemaExprCXX.cpp(5702): error C2248: 'clang::InitializedEntity::InitializedEntity': cannot access private member declared in class 'clang::InitializedEntity'
>> C:\b\slave\sanitizer-windows\llvm\tools\clang\include\clang/Sema/Initialization.h(163): note: see declaration of 'clang::InitializedEntity::InitializedEntity'
>> C:\b\slave\sanitizer-windows\llvm\tools\clang\include\clang/Sema/Initialization.h(40): note: see declaration of 'clang::InitializedEntity'
>> C:\b\slave\sanitizer-windows\llvm\tools\clang\lib\Sema\SemaExprCXX.cpp(5704): error C2326: 'clang::Sema::FindCompositePointerType::Conversion::Conversion(clang::Sema &,clang::SourceLocation,clang::Expr *&,clang::Expr *&,clang::QualType)': function cannot access 'Loc'
>> C:\b\slave\sanitizer-windows\llvm\tools\clang\lib\Sema\SemaExprCXX.cpp(5704): error C2512: 'clang::InitializationKind::InitializationKind': no appropriate default constructor available
>> 251843.519 [4/2/26] Building CXX object lib\Target\X86\CMakeFiles\LLVMX86CodeGen.dir\X86WinAllocaExpander.cpp.obj
>> 251843.902 [4/1/27] Building CXX object lib\Target\X86\CMakeFiles\LLVMX86CodeGen.dir\X86TargetMachine.cpp.obj
>> 251846.937 [4/0/28] Building CXX object lib\Target\X86\CMakeFiles\LLVMX86CodeGen.dir\X86ISelLowering.cpp.obj
>> ninja: build stopped: subcommand failed.
>>
>>
>>
>> On Wed, Oct 19, 2016 at 6:29 PM Richard Smith via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: rsmith
>>> Date: Wed Oct 19 20:20:00 2016
>>> New Revision: 284685
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=284685&view=rev
>>> Log:
>>> Refactor and simplify Sema::FindCompositePointerType. No functionality
>>> change intended.
>>>
>>> Modified:
>>>     cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaE
>>> xprCXX.cpp?rev=284685&r1=284684&r2=284685&view=diff
>>> ============================================================
>>> ==================
>>> --- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Oct 19 20:20:00 2016
>>> @@ -5520,7 +5520,7 @@ QualType Sema::CXXCheckConditionalOperan
>>>  /// \brief Find a merged pointer type and convert the two expressions
>>> to it.
>>>  ///
>>>  /// This finds the composite pointer type (or member pointer type) for
>>> @p E1
>>> -/// and @p E2 according to C++11 5.9p2. It converts both expressions to
>>> this
>>> +/// and @p E2 according to C++1z 5p14. It converts both expressions to
>>> this
>>>  /// type and returns it.
>>>  /// It does not emit diagnostics.
>>>  ///
>>> @@ -5538,69 +5538,87 @@ QualType Sema::FindCompositePointerType(
>>>      *NonStandardCompositeType = false;
>>>
>>>    assert(getLangOpts().CPlusPlus && "This function assumes C++");
>>> +
>>> +  // C++1z [expr]p14:
>>> +  //   The composite pointer type of two operands p1 and p2 having
>>> types T1
>>> +  //   and T2
>>>    QualType T1 = E1->getType(), T2 = E2->getType();
>>>
>>> -  // C++11 5.9p2
>>> -  //   Pointer conversions and qualification conversions are performed
>>> on
>>> -  //   pointer operands to bring them to their composite pointer type.
>>> If
>>> -  //   one operand is a null pointer constant, the composite pointer
>>> type is
>>> -  //   std::nullptr_t if the other operand is also a null pointer
>>> constant or,
>>> -  //   if the other operand is a pointer, the type of the other operand.
>>> -  if (!T1->isAnyPointerType() && !T1->isMemberPointerType() &&
>>> -      !T2->isAnyPointerType() && !T2->isMemberPointerType()) {
>>> -    if (T1->isNullPtrType() &&
>>> -        E2->isNullPointerConstant(Context,
>>> Expr::NPC_ValueDependentIsNull)) {
>>> -      E2 = ImpCastExprToType(E2, T1, CK_NullToPointer).get();
>>> -      return T1;
>>> -    }
>>> -    if (T2->isNullPtrType() &&
>>> -        E1->isNullPointerConstant(Context,
>>> Expr::NPC_ValueDependentIsNull)) {
>>> -      E1 = ImpCastExprToType(E1, T2, CK_NullToPointer).get();
>>> -      return T2;
>>> -    }
>>> +  //   where at least one is a pointer or pointer to member type or
>>> +  //   std::nullptr_t is:
>>> +  bool T1IsPointerLike = T1->isAnyPointerType() ||
>>> T1->isMemberPointerType() ||
>>> +                         T1->isNullPtrType();
>>> +  bool T2IsPointerLike = T2->isAnyPointerType() ||
>>> T2->isMemberPointerType() ||
>>> +                         T2->isNullPtrType();
>>> +  if (!T1IsPointerLike && !T2IsPointerLike)
>>>      return QualType();
>>> -  }
>>>
>>> -  if (E1->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull))
>>> {
>>> -    if (T2->isMemberPointerType())
>>> -      E1 = ImpCastExprToType(E1, T2, CK_NullToMemberPointer).get();
>>> -    else
>>> -      E1 = ImpCastExprToType(E1, T2, CK_NullToPointer).get();
>>> -    return T2;
>>> -  }
>>> -  if (E2->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull))
>>> {
>>> -    if (T1->isMemberPointerType())
>>> -      E2 = ImpCastExprToType(E2, T1, CK_NullToMemberPointer).get();
>>> -    else
>>> -      E2 = ImpCastExprToType(E2, T1, CK_NullToPointer).get();
>>> +  //   - if both p1 and p2 are null pointer constants, std::nullptr_t;
>>> +  // This can't actually happen, following the standard, but we also
>>> use this
>>> +  // to implement the end of [expr.conv], which hits this case.
>>> +  //
>>> +  //   - if either p1 or p2 is a null pointer constant, T2 or T1,
>>> respectively;
>>> +  if (T1IsPointerLike &&
>>> +      E2->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull))
>>> {
>>> +    E2 = ImpCastExprToType(E2, T1, T1->isMemberPointerType()
>>> +                                       ? CK_NullToMemberPointer
>>> +                                       : CK_NullToPointer).get();
>>>      return T1;
>>>    }
>>> +  if (T2IsPointerLike &&
>>> +      E1->isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNull))
>>> {
>>> +    E1 = ImpCastExprToType(E1, T2, T2->isMemberPointerType()
>>> +                                       ? CK_NullToMemberPointer
>>> +                                       : CK_NullToPointer).get();
>>> +    return T2;
>>> +  }
>>>
>>>    // Now both have to be pointers or member pointers.
>>> -  if ((!T1->isPointerType() && !T1->isMemberPointerType()) ||
>>> -      (!T2->isPointerType() && !T2->isMemberPointerType()))
>>> +  if (!T1IsPointerLike || !T2IsPointerLike)
>>>      return QualType();
>>> +  assert(!T1->isNullPtrType() && !T2->isNullPtrType() &&
>>> +         "nullptr_t should be a null pointer constant");
>>>
>>> -  //   Otherwise, of one of the operands has type "pointer to cv1
>>> void," then
>>> -  //   the other has type "pointer to cv2 T" and the composite pointer
>>> type is
>>> -  //   "pointer to cv12 void," where cv12 is the union of cv1 and cv2.
>>> -  //   Otherwise, the composite pointer type is a pointer type similar
>>> to the
>>> -  //   type of one of the operands, with a cv-qualification signature
>>> that is
>>> -  //   the union of the cv-qualification signatures of the operand
>>> types.
>>> -  // In practice, the first part here is redundant; it's subsumed by
>>> the second.
>>> -  // What we do here is, we build the two possible composite types, and
>>> try the
>>> -  // conversions in both directions. If only one works, or if the two
>>> composite
>>> -  // types are the same, we have succeeded.
>>> +  //  - if T1 or T2 is "pointer to cv1 void" and the other type is
>>> +  //    "pointer to cv2 T", "pointer to cv12 void", where cv12 is
>>> +  //    the union of cv1 and cv2;
>>> +  //  - if T1 or T2 is "pointer to noexcept function" and the other
>>> type is
>>> +  //    "pointer to function", where the function types are otherwise
>>> the same,
>>> +  //    "pointer to function";
>>> +  //     FIXME: This rule is defective: it should also permit removing
>>> noexcept
>>> +  //     from a pointer to member function.  As a Clang extension, we
>>> also
>>> +  //     permit removing 'noreturn', so we generalize this rule to;
>>> +  //     - [Clang] If T1 and T2 are both of type "pointer to function"
>>> or
>>> +  //       "pointer to member function" and the pointee types can be
>>> unified
>>> +  //       by a function pointer conversion, that conversion is applied
>>> +  //       before checking the following rules.
>>> +  //  - if T1 is "pointer to cv1 C1" and T2 is "pointer to cv2 C2",
>>> where C1
>>> +  //    is reference-related to C2 or C2 is reference-related to C1
>>> (8.6.3),
>>> +  //    the cv-combined type of T1 and T2 or the cv-combined type of T2
>>> and T1,
>>> +  //    respectively;
>>> +  //  - if T1 is "pointer to member of C1 of type cv1 U1" and T2 is
>>> "pointer
>>> +  //    to member of C2 of type cv2 U2" where C1 is reference-related
>>> to C2 or
>>> +  //    C2 is reference-related to C1 (8.6.3), the cv-combined type of
>>> T2 and
>>> +  //    T1 or the cv-combined type of T1 and T2, respectively;
>>> +  //  - if T1 and T2 are similar types (4.5), the cv-combined type of
>>> T1 and
>>> +  //    T2;
>>> +  //
>>> +  // If looked at in the right way, these bullets all do the same thing.
>>> +  // What we do here is, we build the two possible cv-combined types,
>>> and try
>>> +  // the conversions in both directions. If only one works, or if the
>>> two
>>> +  // composite types are the same, we have succeeded.
>>>    // FIXME: extended qualifiers?
>>> -  typedef SmallVector<unsigned, 4> QualifierVector;
>>> -  QualifierVector QualifierUnion;
>>> -  typedef SmallVector<std::pair<const Type *, const Type *>, 4>
>>> -      ContainingClassVector;
>>> -  ContainingClassVector MemberOfClass;
>>> -  QualType Composite1 = Context.getCanonicalType(T1),
>>> -           Composite2 = Context.getCanonicalType(T2);
>>> +  //
>>> +  // Note that this will fail to find a composite pointer type for
>>> "pointer
>>> +  // to void" and "pointer to function". We can't actually perform the
>>> final
>>> +  // conversion in this case, even though a composite pointer type
>>> formally
>>> +  // exists.
>>> +  SmallVector<unsigned, 4> QualifierUnion;
>>> +  SmallVector<std::pair<const Type *, const Type *>, 4> MemberOfClass;
>>> +  QualType Composite1 = Context.getCanonicalType(T1);
>>> +  QualType Composite2 = Context.getCanonicalType(T2);
>>>    unsigned NeedConstBefore = 0;
>>> -  do {
>>> +  while (true) {
>>>      const PointerType *Ptr1, *Ptr2;
>>>      if ((Ptr1 = Composite1->getAs<PointerType>()) &&
>>>          (Ptr2 = Composite2->getAs<PointerType>())) {
>>> @@ -5642,7 +5660,7 @@ QualType Sema::FindCompositePointerType(
>>>
>>>      // Cannot unwrap any more types.
>>>      break;
>>> -  } while (true);
>>> +  }
>>>
>>>    if (NeedConstBefore && NonStandardCompositeType) {
>>>      // Extension: Add 'const' to qualifiers that come before the first
>>> qualifier
>>> @@ -5657,94 +5675,73 @@ QualType Sema::FindCompositePointerType(
>>>    }
>>>
>>>    // Rewrap the composites as pointers or member pointers with the
>>> union CVRs.
>>> -  ContainingClassVector::reverse_iterator MOC
>>> -    = MemberOfClass.rbegin();
>>> -  for (QualifierVector::reverse_iterator
>>> -         I = QualifierUnion.rbegin(),
>>> -         E = QualifierUnion.rend();
>>> -       I != E; (void)++I, ++MOC) {
>>> -    Qualifiers Quals = Qualifiers::fromCVRMask(*I);
>>> -    if (MOC->first && MOC->second) {
>>> +  auto MOC = MemberOfClass.rbegin();
>>> +  for (unsigned CVR : llvm::reverse(QualifierUnion)) {
>>> +    Qualifiers Quals = Qualifiers::fromCVRMask(CVR);
>>> +    auto Classes = *MOC++;
>>> +    if (Classes.first && Classes.second) {
>>>        // Rebuild member pointer type
>>>        Composite1 = Context.getMemberPointerType(
>>> -                                    Context.getQualifiedType(Composite1,
>>> Quals),
>>> -                                    MOC->first);
>>> +          Context.getQualifiedType(Composite1, Quals), Classes.first);
>>>        Composite2 = Context.getMemberPointerType(
>>> -                                    Context.getQualifiedType(Composite2,
>>> Quals),
>>> -                                    MOC->second);
>>> +          Context.getQualifiedType(Composite2, Quals), Classes.second);
>>>      } else {
>>>        // Rebuild pointer type
>>> -      Composite1
>>> -        = Context.getPointerType(Context.getQualifiedType(Composite1,
>>> Quals));
>>> -      Composite2
>>> -        = Context.getPointerType(Context.getQualifiedType(Composite2,
>>> Quals));
>>> +      Composite1 =
>>> +          Context.getPointerType(Context.getQualifiedType(Composite1,
>>> Quals));
>>> +      Composite2 =
>>> +          Context.getPointerType(Context.getQualifiedType(Composite2,
>>> Quals));
>>>      }
>>>    }
>>>
>>> -  // Try to convert to the first composite pointer type.
>>> -  InitializedEntity Entity1
>>> -    = InitializedEntity::InitializeTemporary(Composite1);
>>> -  InitializationKind Kind
>>> -    = InitializationKind::CreateCopy(Loc, SourceLocation());
>>> -  InitializationSequence E1ToC1(*this, Entity1, Kind, E1);
>>> -  InitializationSequence E2ToC1(*this, Entity1, Kind, E2);
>>> -
>>> -  if (E1ToC1 && E2ToC1) {
>>> -    // Conversion to Composite1 is viable.
>>> -    if (!Context.hasSameType(Composite1, Composite2)) {
>>> -      // Composite2 is a different type from Composite1. Check whether
>>> -      // Composite2 is also viable.
>>> -      InitializedEntity Entity2
>>> -        = InitializedEntity::InitializeTemporary(Composite2);
>>> -      InitializationSequence E1ToC2(*this, Entity2, Kind, E1);
>>> -      InitializationSequence E2ToC2(*this, Entity2, Kind, E2);
>>> -      if (E1ToC2 && E2ToC2) {
>>> -        // Both Composite1 and Composite2 are viable and are different;
>>> -        // this is an ambiguity.
>>> +  struct Conversion {
>>> +    Sema &S;
>>> +    SourceLocation Loc;
>>> +    Expr *&E1, *&E2;
>>> +    QualType Composite;
>>> +    InitializedEntity Entity =
>>> +        InitializedEntity::InitializeTemporary(Composite);
>>> +    InitializationKind Kind =
>>> +        InitializationKind::CreateCopy(Loc, SourceLocation());
>>> +    InitializationSequence E1ToC, E2ToC;
>>> +    bool Viable = E1ToC && E2ToC;
>>> +
>>> +    Conversion(Sema &S, SourceLocation Loc, Expr *&E1, Expr *&E2,
>>> +               QualType Composite)
>>> +        : S(S), Loc(Loc), E1(E1), E2(E2), Composite(Composite),
>>> +          E1ToC(S, Entity, Kind, E1), E2ToC(S, Entity, Kind, E2) {
>>> +    }
>>> +
>>> +    QualType perform() {
>>> +      ExprResult E1Result = E1ToC.Perform(S, Entity, Kind, E1);
>>> +      if (E1Result.isInvalid())
>>>          return QualType();
>>> -      }
>>> -    }
>>> +      E1 = E1Result.getAs<Expr>();
>>>
>>> -    // Convert E1 to Composite1
>>> -    ExprResult E1Result
>>> -      = E1ToC1.Perform(*this, Entity1, Kind, E1);
>>> -    if (E1Result.isInvalid())
>>> -      return QualType();
>>> -    E1 = E1Result.getAs<Expr>();
>>> -
>>> -    // Convert E2 to Composite1
>>> -    ExprResult E2Result
>>> -      = E2ToC1.Perform(*this, Entity1, Kind, E2);
>>> -    if (E2Result.isInvalid())
>>> -      return QualType();
>>> -    E2 = E2Result.getAs<Expr>();
>>> -
>>> -    return Composite1;
>>> -  }
>>> -
>>> -  // Check whether Composite2 is viable.
>>> -  InitializedEntity Entity2
>>> -    = InitializedEntity::InitializeTemporary(Composite2);
>>> -  InitializationSequence E1ToC2(*this, Entity2, Kind, E1);
>>> -  InitializationSequence E2ToC2(*this, Entity2, Kind, E2);
>>> -  if (!E1ToC2 || !E2ToC2)
>>> -    return QualType();
>>> +      ExprResult E2Result = E2ToC.Perform(S, Entity, Kind, E2);
>>> +      if (E2Result.isInvalid())
>>> +        return QualType();
>>> +      E2 = E2Result.getAs<Expr>();
>>>
>>> -  // Convert E1 to Composite2
>>> -  ExprResult E1Result
>>> -    = E1ToC2.Perform(*this, Entity2, Kind, E1);
>>> -  if (E1Result.isInvalid())
>>> -    return QualType();
>>> -  E1 = E1Result.getAs<Expr>();
>>> +      return Composite;
>>> +    }
>>> +  };
>>>
>>> -  // Convert E2 to Composite2
>>> -  ExprResult E2Result
>>> -    = E2ToC2.Perform(*this, Entity2, Kind, E2);
>>> -  if (E2Result.isInvalid())
>>> +  // Try to convert to each composite pointer type.
>>> +  Conversion C1(*this, Loc, E1, E2, Composite1);
>>> +  if (C1.Viable && Context.hasSameType(Composite1, Composite2))
>>> +    return C1.perform();
>>> +  Conversion C2(*this, Loc, E1, E2, Composite2);
>>> +
>>> +  if (C1.Viable == C2.Viable) {
>>> +    // Either Composite1 and Composite2 are viable and are different, or
>>> +    // neither is viable.
>>> +    // FIXME: How both be viable and different?
>>>      return QualType();
>>> -  E2 = E2Result.getAs<Expr>();
>>> +  }
>>>
>>> -  return Composite2;
>>> +  // Convert to the chosen type.
>>> +  return (C1.Viable ? C1 : C2).perform();
>>>  }
>>>
>>>  ExprResult Sema::MaybeBindToTemporary(Expr *E) {
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>> --
>> Mike
>> Sent from phone
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161020/d195f696/attachment-0001.html>


More information about the cfe-commits mailing list