[cfe-commits] r97090 - in /cfe/trunk: lib/Sema/SemaDeclCXX.cpp lib/Sema/SemaExprCXX.cpp lib/Sema/SemaOverload.cpp lib/Sema/SemaOverload.h test/SemaCXX/overload-call.cpp

John McCall rjmccall at apple.com
Wed Feb 24 17:37:25 PST 2010


Author: rjmccall
Date: Wed Feb 24 19:37:24 2010
New Revision: 97090

URL: http://llvm.org/viewvc/llvm-project?rev=97090&view=rev
Log:
Catch more uses of uninitialized implicit conversion sequences.
When diagnosing bad conversions, skip the conversion for ignored object
arguments.  Fixes PR 6398.


Modified:
    cfe/trunk/lib/Sema/SemaDeclCXX.cpp
    cfe/trunk/lib/Sema/SemaExprCXX.cpp
    cfe/trunk/lib/Sema/SemaOverload.cpp
    cfe/trunk/lib/Sema/SemaOverload.h
    cfe/trunk/test/SemaCXX/overload-call.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=97090&r1=97089&r2=97090&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Wed Feb 24 19:37:24 2010
@@ -4395,8 +4395,7 @@
 
   // Most paths end in a failed conversion.
   if (ICS) {
-    ICS->setBad();
-    ICS->Bad.init(BadConversionSequence::no_conversion, Init, DeclType);
+    ICS->setBad(BadConversionSequence::no_conversion, Init, DeclType);
   }
 
   // C++ [dcl.init.ref]p5:

Modified: cfe/trunk/lib/Sema/SemaExprCXX.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExprCXX.cpp?rev=97090&r1=97089&r2=97090&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaExprCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExprCXX.cpp Wed Feb 24 19:37:24 2010
@@ -1344,8 +1344,7 @@
                                 AssignmentAction Action, bool AllowExplicit,
                                 bool Elidable,
                                 ImplicitConversionSequence& ICS) {
-  ICS.setBad();
-  ICS.Bad.init(BadConversionSequence::no_conversion, From, ToType);
+  ICS.setBad(BadConversionSequence::no_conversion, From, ToType);
   if (Elidable && getLangOptions().CPlusPlus0x) {
     ICS = TryImplicitConversion(From, ToType,
                                 /*SuppressUserConversions=*/false,
@@ -1759,6 +1758,7 @@
     return ICS.UserDefined.After.getToType(2);
   case ImplicitConversionSequence::AmbiguousConversion:
     return ICS.Ambiguous.getToType();
+
   case ImplicitConversionSequence::EllipsisConversion:
   case ImplicitConversionSequence::BadConversion:
     llvm_unreachable("function not valid for ellipsis or bad conversions");
@@ -1802,7 +1802,7 @@
         return false;
     }
   }
-  ICS.setBad();
+
   //   -- If E2 is an rvalue, or if the conversion above cannot be done:
   //      -- if E1 and E2 have class type, and the underlying class types are
   //         the same or one is a base class of the other:
@@ -1816,14 +1816,22 @@
     //         E1 can be converted to match E2 if the class of T2 is the
     //         same type as, or a base class of, the class of T1, and
     //         [cv2 > cv1].
-    if ((FRec == TRec || FDerivedFromT) && TTy.isAtLeastAsQualifiedAs(FTy)) {
-      // Could still fail if there's no copy constructor.
-      // FIXME: Is this a hard error then, or just a conversion failure? The
-      // standard doesn't say.
-      ICS = Self.TryCopyInitialization(From, TTy,
-                                       /*SuppressUserConversions=*/false,
-                                       /*ForceRValue=*/false,
-                                       /*InOverloadResolution=*/false);
+    if (FRec == TRec || FDerivedFromT) {
+      if (TTy.isAtLeastAsQualifiedAs(FTy)) {
+        // Could still fail if there's no copy constructor.
+        // FIXME: Is this a hard error then, or just a conversion failure? The
+        // standard doesn't say.
+        ICS = Self.TryCopyInitialization(From, TTy,
+                                         /*SuppressUserConversions=*/false,
+                                         /*ForceRValue=*/false,
+                                         /*InOverloadResolution=*/false);
+      } else {
+        ICS.setBad(BadConversionSequence::bad_qualifiers, From, TTy);
+      }
+    } else {
+      // Can't implicitly convert FTy to a derived class TTy.
+      // TODO: more specific error for this.
+      ICS.setBad(BadConversionSequence::no_conversion, From, TTy);
     }
   } else {
     //     -- Otherwise: E1 can be converted to match E2 if E1 can be
@@ -2212,9 +2220,8 @@
                           /*ForceRValue=*/false,
                           /*InOverloadResolution=*/false);
 
+  bool ToC2Viable = false;
   ImplicitConversionSequence E1ToC2, E2ToC2;
-  E1ToC2.setBad();
-  E2ToC2.setBad();  
   if (Context.getCanonicalType(Composite1) !=
       Context.getCanonicalType(Composite2)) {
     E1ToC2 = TryImplicitConversion(E1, Composite2,
@@ -2227,10 +2234,10 @@
                                    /*AllowExplicit=*/false,
                                    /*ForceRValue=*/false,
                                    /*InOverloadResolution=*/false);
+    ToC2Viable = !E1ToC2.isBad() && !E2ToC2.isBad();
   }
 
   bool ToC1Viable = !E1ToC1.isBad() && !E2ToC1.isBad();
-  bool ToC2Viable = !E1ToC2.isBad() && !E2ToC2.isBad();
   if (ToC1Viable && !ToC2Viable) {
     if (!PerformImplicitConversion(E1, Composite1, E1ToC1, Sema::AA_Converting) &&
         !PerformImplicitConversion(E2, Composite1, E2ToC1, Sema::AA_Converting))

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=97090&r1=97089&r2=97090&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Wed Feb 24 19:37:24 2010
@@ -453,8 +453,7 @@
   }
 
   if (!getLangOptions().CPlusPlus) {
-    ICS.setBad();
-    ICS.Bad.init(BadConversionSequence::no_conversion, From, ToType);
+    ICS.setBad(BadConversionSequence::no_conversion, From, ToType);
     return ICS;
   }
 
@@ -500,8 +499,7 @@
     //   13.3.1.6 in all cases, only standard conversion sequences and
     //   ellipsis conversion sequences are allowed.
     if (SuppressUserConversions && ICS.isUserDefined()) {
-      ICS.setBad();
-      ICS.Bad.init(BadConversionSequence::suppressed_user, From, ToType);
+      ICS.setBad(BadConversionSequence::suppressed_user, From, ToType);
     }
   } else if (UserDefResult == OR_Ambiguous && !SuppressUserConversions) {
     ICS.setAmbiguous();
@@ -512,8 +510,7 @@
       if (Cand->Viable)
         ICS.Ambiguous.addConversion(Cand->Function);
   } else {
-    ICS.setBad();
-    ICS.Bad.init(BadConversionSequence::no_conversion, From, ToType);
+    ICS.setBad(BadConversionSequence::no_conversion, From, ToType);
   }
 
   return ICS;
@@ -2196,7 +2193,7 @@
                             bool InOverloadResolution) {
   if (ToType->isReferenceType()) {
     ImplicitConversionSequence ICS;
-    ICS.Bad.init(BadConversionSequence::no_conversion, From, ToType);
+    ICS.setBad(BadConversionSequence::no_conversion, From, ToType);
     CheckReferenceInit(From, ToType,
                        /*FIXME:*/From->getLocStart(),
                        SuppressUserConversions,
@@ -2268,8 +2265,6 @@
   // Set up the conversion sequence as a "bad" conversion, to allow us
   // to exit early.
   ImplicitConversionSequence ICS;
-  ICS.Standard.setAsIdentityConversion();
-  ICS.setBad();
 
   // We need to have an object of class type.
   QualType FromType = OrigFromType;
@@ -2293,25 +2288,29 @@
   if (ImplicitParamType.getCVRQualifiers() 
                                     != FromTypeCanon.getLocalCVRQualifiers() &&
       !ImplicitParamType.isAtLeastAsQualifiedAs(FromTypeCanon)) {
-    ICS.Bad.init(BadConversionSequence::bad_qualifiers,
-                 OrigFromType, ImplicitParamType);
+    ICS.setBad(BadConversionSequence::bad_qualifiers,
+               OrigFromType, ImplicitParamType);
     return ICS;
   }
 
   // Check that we have either the same type or a derived type. It
   // affects the conversion rank.
   QualType ClassTypeCanon = Context.getCanonicalType(ClassType);
-  if (ClassTypeCanon == FromTypeCanon.getLocalUnqualifiedType())
-    ICS.Standard.Second = ICK_Identity;
-  else if (IsDerivedFrom(FromType, ClassType))
-    ICS.Standard.Second = ICK_Derived_To_Base;
+  ImplicitConversionKind SecondKind;
+  if (ClassTypeCanon == FromTypeCanon.getLocalUnqualifiedType()) {
+    SecondKind = ICK_Identity;
+  } else if (IsDerivedFrom(FromType, ClassType))
+    SecondKind = ICK_Derived_To_Base;
   else {
-    ICS.Bad.init(BadConversionSequence::unrelated_class, FromType, ImplicitParamType);
+    ICS.setBad(BadConversionSequence::unrelated_class,
+               FromType, ImplicitParamType);
     return ICS;
   }
 
   // Success. Mark this as a reference binding.
   ICS.setStandard();
+  ICS.Standard.setAsIdentityConversion();
+  ICS.Standard.Second = SecondKind;
   ICS.Standard.setFromType(FromType);
   ICS.Standard.setAllToTypes(ImplicitParamType);
   ICS.Standard.ReferenceBinding = true;
@@ -4464,7 +4463,7 @@
   QualType ToTy = Conv.Bad.getToType();
 
   if (FromTy == S.Context.OverloadTy) {
-    assert(FromExpr);
+    assert(FromExpr && "overload set argument came from implicit argument?");
     Expr *E = FromExpr->IgnoreParens();
     if (isa<UnaryOperator>(E))
       E = cast<UnaryOperator>(E)->getSubExpr()->IgnoreParens();
@@ -4667,8 +4666,9 @@
   case ovl_fail_bad_final_conversion:
     return S.NoteOverloadCandidate(Fn);
 
-  case ovl_fail_bad_conversion:
-    for (unsigned I = 0, N = Cand->Conversions.size(); I != N; ++I)
+  case ovl_fail_bad_conversion: {
+    unsigned I = (Cand->IgnoreObjectArgument ? 1 : 0);
+    for (unsigned N = Cand->Conversions.size(); I != N; ++I)
       if (Cand->Conversions[I].isBad())
         return DiagnoseBadConversion(S, Cand, I);
     
@@ -4677,6 +4677,7 @@
     // those conditions and diagnose them well.
     return S.NoteOverloadCandidate(Fn);
   }
+  }
 }
 
 void NoteSurrogateCandidate(Sema &S, OverloadCandidate *Cand) {
@@ -4842,7 +4843,7 @@
   if (Cand->FailureKind != ovl_fail_bad_conversion) return;
 
   // Skip forward to the first bad conversion.
-  unsigned ConvIdx = 0;
+  unsigned ConvIdx = (Cand->IgnoreObjectArgument ? 1 : 0);
   unsigned ConvCount = Cand->Conversions.size();
   while (true) {
     assert(ConvIdx != ConvCount && "no bad conversion in candidate");
@@ -4854,6 +4855,9 @@
   if (ConvIdx == ConvCount)
     return;
 
+  assert(!Cand->Conversions[ConvIdx].isInitialized() &&
+         "remaining conversion is initialized?");
+
   // FIXME: these should probably be preserved from the overload
   // operation somehow.
   bool SuppressUserConversions = false;

Modified: cfe/trunk/lib/Sema/SemaOverload.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.h?rev=97090&r1=97089&r2=97090&view=diff
==============================================================================
--- cfe/trunk/lib/Sema/SemaOverload.h (original)
+++ cfe/trunk/lib/Sema/SemaOverload.h Wed Feb 24 19:37:24 2010
@@ -316,14 +316,22 @@
     };
 
   private:
+    enum {
+      Uninitialized = BadConversion + 1
+    };
+
     /// ConversionKind - The kind of implicit conversion sequence.
-    Kind ConversionKind;
+    unsigned ConversionKind;
 
     void setKind(Kind K) {
-      if (isAmbiguous()) Ambiguous.destruct();
+      destruct();
       ConversionKind = K;
     }
 
+    void destruct() {
+      if (ConversionKind == AmbiguousConversion) Ambiguous.destruct();
+    }
+
   public:
     union {
       /// When ConversionKind == StandardConversion, provides the
@@ -343,14 +351,15 @@
       BadConversionSequence Bad;
     };
 
-    ImplicitConversionSequence() : ConversionKind(BadConversion) {}
+    ImplicitConversionSequence() : ConversionKind(Uninitialized) {}
     ~ImplicitConversionSequence() {
-      if (isAmbiguous()) Ambiguous.destruct();
+      destruct();
     }
     ImplicitConversionSequence(const ImplicitConversionSequence &Other)
       : ConversionKind(Other.ConversionKind)
     {
       switch (ConversionKind) {
+      case Uninitialized: break;
       case StandardConversion: Standard = Other.Standard; break;
       case UserDefinedConversion: UserDefined = Other.UserDefined; break;
       case AmbiguousConversion: Ambiguous.copyFrom(Other.Ambiguous); break;
@@ -361,26 +370,45 @@
 
     ImplicitConversionSequence &
         operator=(const ImplicitConversionSequence &Other) {
-      if (isAmbiguous()) Ambiguous.destruct();
+      destruct();
       new (this) ImplicitConversionSequence(Other);
       return *this;
     }
     
-    Kind getKind() const { return ConversionKind; }
-    bool isBad() const { return ConversionKind == BadConversion; }
-    bool isStandard() const { return ConversionKind == StandardConversion; }
-    bool isEllipsis() const { return ConversionKind == EllipsisConversion; }
-    bool isAmbiguous() const { return ConversionKind == AmbiguousConversion; }
-    bool isUserDefined() const {
-      return ConversionKind == UserDefinedConversion;
+    Kind getKind() const {
+      assert(isInitialized() && "querying uninitialized conversion");
+      return Kind(ConversionKind);
+    }
+    bool isBad() const { return getKind() == BadConversion; }
+    bool isStandard() const { return getKind() == StandardConversion; }
+    bool isEllipsis() const { return getKind() == EllipsisConversion; }
+    bool isAmbiguous() const { return getKind() == AmbiguousConversion; }
+    bool isUserDefined() const { return getKind() == UserDefinedConversion; }
+
+    /// Determines whether this conversion sequence has been
+    /// initialized.  Most operations should never need to query
+    /// uninitialized conversions and should assert as above.
+    bool isInitialized() const { return ConversionKind != Uninitialized; }
+
+    /// Sets this sequence as a bad conversion for an explicit argument.
+    void setBad(BadConversionSequence::FailureKind Failure,
+                Expr *FromExpr, QualType ToType) {
+      setKind(BadConversion);
+      Bad.init(Failure, FromExpr, ToType);
+    }
+
+    /// Sets this sequence as a bad conversion for an implicit argument.
+    void setBad(BadConversionSequence::FailureKind Failure,
+                QualType FromType, QualType ToType) {
+      setKind(BadConversion);
+      Bad.init(Failure, FromType, ToType);
     }
 
-    void setBad() { setKind(BadConversion); }
     void setStandard() { setKind(StandardConversion); }
     void setEllipsis() { setKind(EllipsisConversion); }
     void setUserDefined() { setKind(UserDefinedConversion); }
     void setAmbiguous() {
-      if (isAmbiguous()) return;
+      if (ConversionKind == AmbiguousConversion) return;
       ConversionKind = AmbiguousConversion;
       Ambiguous.construct();
     }
@@ -490,6 +518,7 @@
     bool hasAmbiguousConversion() const {
       for (llvm::SmallVectorImpl<ImplicitConversionSequence>::const_iterator
              I = Conversions.begin(), E = Conversions.end(); I != E; ++I) {
+        if (!I->isInitialized()) return false;
         if (I->isAmbiguous()) return true;
       }
       return false;

Modified: cfe/trunk/test/SemaCXX/overload-call.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/overload-call.cpp?rev=97090&r1=97089&r2=97090&view=diff
==============================================================================
--- cfe/trunk/test/SemaCXX/overload-call.cpp (original)
+++ cfe/trunk/test/SemaCXX/overload-call.cpp Wed Feb 24 19:37:24 2010
@@ -359,3 +359,16 @@
     int &ir = f(b);
   }
 }
+
+// PR 6398
+namespace test4 {
+  class A;
+  class B {
+    static void foo(); // expected-note {{not viable}}
+    static void foo(int*); // expected-note {{not viable}}
+
+    void bar(A *a) { 
+      foo(a); // expected-error {{no matching function for call}}
+    }
+  };
+}





More information about the cfe-commits mailing list