[cfe-commits] r58873 - in /cfe/trunk: include/clang/Basic/DiagnosticKinds.def lib/Sema/Sema.h lib/Sema/SemaNamedCast.cpp test/SemaCXX/static-cast.cpp

Sebastian Redl sebastian.redl at getdesigned.at
Fri Nov 7 15:29:29 PST 2008


Author: cornedbee
Date: Fri Nov  7 17:29:29 2008
New Revision: 58873

URL: http://llvm.org/viewvc/llvm-project?rev=58873&view=rev
Log:
Greatly improve static_cast diagnostics

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticKinds.def
    cfe/trunk/lib/Sema/Sema.h
    cfe/trunk/lib/Sema/SemaNamedCast.cpp
    cfe/trunk/test/SemaCXX/static-cast.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticKinds.def
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticKinds.def?rev=58873&r1=58872&r2=58873&view=diff

==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticKinds.def (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticKinds.def Fri Nov  7 17:29:29 2008
@@ -1150,6 +1150,10 @@
      "'%0' is not a pointer")
 DIAG(err_bad_dynamic_cast_not_polymorphic, ERROR,
      "'%0' is not polymorphic")
+DIAG(err_ambiguous_base_to_derived_cast, ERROR,
+     "ambiguous static_cast from base '%0' to derived '%1':%2")
+DIAG(err_static_downcast_via_virtual, ERROR,
+     "cannot cast '%0' to '%1' via virtual base '%2'")
 
 DIAG(err_invalid_use_of_function_type, ERROR,
      "a function type is not allowed here")

Modified: cfe/trunk/lib/Sema/Sema.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.h?rev=58873&r1=58872&r2=58873&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/Sema.h (original)
+++ cfe/trunk/lib/Sema/Sema.h Fri Nov  7 17:29:29 2008
@@ -750,12 +750,27 @@
   void CheckDynamicCast(Expr *&SrcExpr, QualType DestType,
                         const SourceRange &OpRange,
                         const SourceRange &DestRange);
+
+  enum TryStaticCastResult {
+    TSC_NotApplicable, ///< The cast method is not applicable.
+    TSC_Success,       ///< The cast method is appropriate and successful.
+    TSC_Failed         ///< The cast method is appropriate, but failed. A
+                       ///< diagnostic has been emitted.
+  };
+
   bool CastsAwayConstness(QualType SrcType, QualType DestType);
-  bool IsStaticReferenceDowncast(Expr *SrcExpr, QualType DestType);
-  bool IsStaticPointerDowncast(QualType SrcType, QualType DestType);
-  bool IsStaticDowncast(QualType SrcType, QualType DestType);
-  ImplicitConversionSequence TryDirectInitialization(Expr *SrcExpr,
-                                                     QualType DestType);
+  TryStaticCastResult TryStaticReferenceDowncast(Expr *SrcExpr,
+                                                QualType DestType,
+                                                const SourceRange &OpRange);
+  TryStaticCastResult TryStaticPointerDowncast(QualType SrcType,
+                                              QualType DestType,
+                                              const SourceRange &OpRange);
+  TryStaticCastResult TryStaticDowncast(QualType SrcType, QualType DestType,
+                                       const SourceRange &OpRange,
+                                       QualType OrigSrcType,
+                                       QualType OrigDestType);
+  TryStaticCastResult TryStaticImplicitCast(Expr *SrcExpr, QualType DestType,
+                                           const SourceRange &OpRange);
 
 public:
   //// ActOnCXXThis -  Parse 'this' pointer.

Modified: cfe/trunk/lib/Sema/SemaNamedCast.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaNamedCast.cpp?rev=58873&r1=58872&r2=58873&view=diff

==============================================================================
--- cfe/trunk/lib/Sema/SemaNamedCast.cpp (original)
+++ cfe/trunk/lib/Sema/SemaNamedCast.cpp Fri Nov  7 17:29:29 2008
@@ -17,6 +17,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/Basic/Diagnostic.h"
 #include "llvm/ADT/SmallVector.h"
+#include <set>
 using namespace clang;
 
 /// ActOnCXXNamedCast - Parse {dynamic,static,reinterpret,const}_cast's.
@@ -357,23 +358,14 @@
   // C++ 5.2.9p5, reference downcast.
   // See the function for details.
   // DR 427 specifies that this is to be applied before paragraph 2.
-  if (IsStaticReferenceDowncast(SrcExpr, DestType)) {
+  if (TryStaticReferenceDowncast(SrcExpr, DestType, OpRange)
+      > TSC_NotApplicable) {
     return;
   }
 
   // C++ 5.2.9p2: An expression e can be explicitly converted to a type T
   //   [...] if the declaration "T t(e);" is well-formed, [...].
-  ImplicitConversionSequence ICS = TryDirectInitialization(SrcExpr, DestType);
-  if (ICS.ConversionKind != ImplicitConversionSequence::BadConversion) {
-    assert(ICS.ConversionKind != ImplicitConversionSequence::EllipsisConversion
-      && "Direct initialization cannot result in ellipsis conversion");
-    // UserDefinedConversionSequence has a StandardConversionSequence as a
-    // prefix. Accessing Standard is therefore safe.
-    // FIXME: Of course, this is definitely not enough.
-    if(ICS.Standard.First != ICK_Identity) {
-      DefaultFunctionArrayConversion(SrcExpr);
-    }
-    // FIXME: Test the details, such as accessible base.
+  if (TryStaticImplicitCast(SrcExpr, DestType, OpRange) > TSC_NotApplicable) {
     return;
   }
 
@@ -408,7 +400,8 @@
 
   // Reverse pointer upcast. C++ 4.10p3 specifies pointer upcast.
   // C++ 5.2.9p8 additionally disallows a cast path through virtual inheritance.
-  if (IsStaticPointerDowncast(SrcType, DestType)) {
+  if (TryStaticPointerDowncast(SrcType, DestType, OpRange)
+      > TSC_NotApplicable) {
     return;
   }
 
@@ -448,8 +441,9 @@
 }
 
 /// Tests whether a conversion according to C++ 5.2.9p5 is valid.
-bool
-Sema::IsStaticReferenceDowncast(Expr *SrcExpr, QualType DestType)
+Sema::TryStaticCastResult
+Sema::TryStaticReferenceDowncast(Expr *SrcExpr, QualType DestType,
+                                const SourceRange &OpRange)
 {
   // C++ 5.2.9p5: An lvalue of type "cv1 B", where B is a class type, can be
   //   cast to type "reference to cv2 D", where D is a class derived from B,
@@ -461,21 +455,23 @@
   // conversion as well.
 
   if (SrcExpr->isLvalue(Context) != Expr::LV_Valid) {
-    return false;
+    return TSC_NotApplicable;
   }
 
   const ReferenceType *DestReference = DestType->getAsReferenceType();
   if (!DestReference) {
-    return false;
+    return TSC_NotApplicable;
   }
   QualType DestPointee = DestReference->getPointeeType();
 
-  return IsStaticDowncast(SrcExpr->getType(), DestPointee);
+  return TryStaticDowncast(SrcExpr->getType(), DestPointee, OpRange,
+                          SrcExpr->getType(), DestType);
 }
 
 /// Tests whether a conversion according to C++ 5.2.9p8 is valid.
-bool
-Sema::IsStaticPointerDowncast(QualType SrcType, QualType DestType)
+Sema::TryStaticCastResult
+Sema::TryStaticPointerDowncast(QualType SrcType, QualType DestType,
+                              const SourceRange &OpRange)
 {
   // C++ 5.2.9p8: An rvalue of type "pointer to cv1 B", where B is a class
   //   type, can be converted to an rvalue of type "pointer to cv2 D", where D
@@ -487,80 +483,131 @@
 
   const PointerType *SrcPointer = SrcType->getAsPointerType();
   if (!SrcPointer) {
-    return false;
+    return TSC_NotApplicable;
   }
 
   const PointerType *DestPointer = DestType->getAsPointerType();
   if (!DestPointer) {
-    return false;
+    return TSC_NotApplicable;
   }
 
-  return IsStaticDowncast(SrcPointer->getPointeeType(),
-                          DestPointer->getPointeeType());
+  return TryStaticDowncast(SrcPointer->getPointeeType(),
+                          DestPointer->getPointeeType(),
+                          OpRange, SrcType, DestType);
 }
 
-/// IsStaticDowncast - Common functionality of IsStaticReferenceDowncast and
-/// IsStaticPointerDowncast. Tests whether a static downcast from SrcType to
+/// TryStaticDowncast - Common functionality of TryStaticReferenceDowncast and
+/// TryStaticPointerDowncast. Tests whether a static downcast from SrcType to
 /// DestType, both of which must be canonical, is possible and allowed.
-bool
-Sema::IsStaticDowncast(QualType SrcType, QualType DestType)
+Sema::TryStaticCastResult
+Sema::TryStaticDowncast(QualType SrcType, QualType DestType,
+                       const SourceRange &OpRange, QualType OrigSrcType,
+                       QualType OrigDestType)
 {
   // Downcast can only happen in class hierarchies, so we need classes.
   if (!DestType->isRecordType() || !SrcType->isRecordType()) {
-    return false;
-  }
-
-  // Comparing cv is cheaper, so do it first.
-  if (!DestType.isAtLeastAsQualifiedAs(SrcType)) {
-    return false;
+    return TSC_NotApplicable;
   }
 
   BasePaths Paths(/*FindAmbiguities=*/true, /*RecordPaths=*/false,
                   /*DetectVirtual=*/true);
   if (!IsDerivedFrom(DestType, SrcType, Paths)) {
-    return false;
+    return TSC_NotApplicable;
+  }
+
+  // Target type does derive from source type. Now we're serious. If an error
+  // appears now, it's not ignored.
+  // This may not be entirely in line with the standard. Take for example:
+  // struct A {};
+  // struct B : virtual A {
+  //   B(A&);
+  // };
+  // 
+  // void f()
+  // {
+  //   (void)static_cast<const B&>(*((A*)0));
+  // }
+  // As far as the standard is concerned, p5 does not apply (A is virtual), so
+  // p2 should be used instead - "const B& t(*((A*)0));" is perfectly valid.
+  // However, both GCC and Comeau reject this example, and accepting it would
+  // mean more complex code if we're to preserve the nice error message.
+  // FIXME: Being 100% compliant here would be nice to have.
+
+  // Must preserve cv, as always.
+  if (!DestType.isAtLeastAsQualifiedAs(SrcType)) {
+    Diag(OpRange.getBegin(), diag::err_bad_cxx_cast_const_away,
+      "static_cast", OrigDestType.getAsString(), OrigSrcType.getAsString(),
+      OpRange);
+    return TSC_Failed;
   }
 
   if (Paths.isAmbiguous(SrcType.getUnqualifiedType())) {
-    return false;
+    // This code is analoguous to that in CheckDerivedToBaseConversion, except
+    // that it builds the paths in reverse order.
+    // To sum up: record all paths to the base and build a nice string from
+    // them. Use it to spice up the error message.
+    Paths.clear();
+    Paths.setRecordingPaths(true);
+    IsDerivedFrom(DestType, SrcType, Paths);
+    std::string PathDisplayStr;
+    std::set<unsigned> DisplayedPaths;
+    for (BasePaths::paths_iterator Path = Paths.begin();
+         Path != Paths.end(); ++Path) {
+      if (DisplayedPaths.insert(Path->back().SubobjectNumber).second) {
+        // We haven't displayed a path to this particular base
+        // class subobject yet.
+        PathDisplayStr += "\n    ";
+        for (BasePath::const_reverse_iterator Element = Path->rbegin();
+             Element != Path->rend(); ++Element)
+          PathDisplayStr += Element->Base->getType().getAsString() + " -> ";
+        PathDisplayStr += DestType.getAsString();
+      }
+    }
+
+    Diag(OpRange.getBegin(), diag::err_ambiguous_base_to_derived_cast,
+      SrcType.getUnqualifiedType().getAsString(),
+      DestType.getUnqualifiedType().getAsString(),
+      PathDisplayStr, OpRange);
+    return TSC_Failed;
   }
 
   if (Paths.getDetectedVirtual() != 0) {
-    return false;
+    QualType VirtualBase(Paths.getDetectedVirtual(), 0);
+    Diag(OpRange.getBegin(), diag::err_static_downcast_via_virtual,
+      OrigSrcType.getAsString(), OrigDestType.getAsString(),
+      VirtualBase.getAsString(), OpRange);
+    return TSC_Failed;
   }
 
   // FIXME: Test accessibility.
 
-  return true;
+  return TSC_Success;
 }
 
-/// TryDirectInitialization - Attempt to direct-initialize a value of the
-/// given type (DestType) from the given expression (SrcExpr), as one would
-/// do when creating an object with new with parameters. This function returns
-/// an implicit conversion sequence that can be used to perform the
-/// initialization.
-/// This routine is very similar to TryCopyInitialization; the differences
-/// between the two (C++ 8.5p12 and C++ 8.5p14) are:
-/// 1) In direct-initialization, all constructors of the target type are
-///    considered, including those marked as explicit.
-/// 2) In direct-initialization, overload resolution is performed over the
-///    constructors of the target type. In copy-initialization, overload
-///    resolution is performed over all conversion functions that result in
-///    the target type. This can lead to different functions used.
-ImplicitConversionSequence
-Sema::TryDirectInitialization(Expr *SrcExpr, QualType DestType)
+/// TryStaticImplicitCast - Tests whether a conversion according to C++ 5.2.9p2
+/// is valid:
+///
+///   An expression e can be explicitly converted to a type T using a
+///   @c static_cast if the declaration "T t(e);" is well-formed [...].
+Sema::TryStaticCastResult
+Sema::TryStaticImplicitCast(Expr *SrcExpr, QualType DestType,
+                           const SourceRange &OpRange)
 {
-  if (!DestType->isRecordType()) {
-    // For non-class types, copy and direct initialization are identical.
-    // C++ 8.5p11
-    // FIXME: Those parts should be in a common function, actually.
-    return TryCopyInitialization(SrcExpr, DestType);
+  if (DestType->isReferenceType()) {
+    // At this point of CheckStaticCast, if the destination is a reference,
+    // this has to work. There is no other way that works.
+    return CheckReferenceInit(SrcExpr, DestType) ? TSC_Failed : TSC_Success;
+  }
+  if (DestType->isRecordType()) {
+    // FIXME: Use an implementation of C++ [over.match.ctor] for this.
+    return TSC_NotApplicable;
   }
 
-  // FIXME: Not enough support for the rest yet, actually.
-  ImplicitConversionSequence ICS;
-  ICS.ConversionKind = ImplicitConversionSequence::BadConversion;
-  return ICS;
+  // FIXME: To get a proper error from invalid conversions here, we need to
+  // reimplement more of this.
+  ImplicitConversionSequence ICS = TryImplicitConversion(SrcExpr, DestType);
+  return ICS.ConversionKind == ImplicitConversionSequence::BadConversion ?
+    TSC_NotApplicable : TSC_Success;
 }
 
 /// CheckDynamicCast - Check that a dynamic_cast\<DestType\>(SrcExpr) is valid.

Modified: cfe/trunk/test/SemaCXX/static-cast.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/static-cast.cpp?rev=58873&r1=58872&r2=58873&view=diff

==============================================================================
--- cfe/trunk/test/SemaCXX/static-cast.cpp (original)
+++ cfe/trunk/test/SemaCXX/static-cast.cpp Fri Nov  7 17:29:29 2008
@@ -37,7 +37,6 @@
   (void)static_cast<void*>((int*)0);
   (void)static_cast<volatile const void*>((const int*)0);
   (void)static_cast<A*>((B*)0);
-  // TryCopyInitialization doesn't handle references yet.
   (void)static_cast<A&>(*((B*)0));
   (void)static_cast<const B*>((C1*)0);
   (void)static_cast<B&>(*((C1*)0));
@@ -53,7 +52,7 @@
   //(void)static_cast<A*>((H*)0); // {{static_cast from 'struct H *' to 'struct A *' is not allowed}}
   (void)static_cast<int>((int*)0); // expected-error {{static_cast from 'int *' to 'int' is not allowed}}
   (void)static_cast<A**>((B**)0); // expected-error {{static_cast from 'struct B **' to 'struct A **' is not allowed}}
-  (void)static_cast<char&>(i); // expected-error {{static_cast from 'int' to 'char &' is not allowed}}
+  (void)static_cast<char&>(i); // expected-error {{non-const reference to type 'char' cannot be initialized with a value of type 'int'}}
 }
 
 // Anything to void
@@ -73,19 +72,19 @@
 
   // Bad code below
 
-  (void)static_cast<C1*>((A*)0); // expected-error {{static_cast from 'struct A *' to 'struct C1 *' is not allowed}}
-  (void)static_cast<C1&>(*((A*)0)); // expected-error {{static_cast from 'struct A' to 'struct C1 &' is not allowed}}
-  (void)static_cast<D*>((A*)0); // expected-error {{static_cast from 'struct A *' to 'struct D *' is not allowed}}
-  (void)static_cast<D&>(*((A*)0)); // expected-error {{static_cast from 'struct A' to 'struct D &' is not allowed}}
-  (void)static_cast<B*>((const A*)0); // expected-error {{static_cast from 'struct A const *' to 'struct B *' is not allowed}}
-  (void)static_cast<B&>(*((const A*)0)); // expected-error {{static_cast from 'struct A const' to 'struct B &' is not allowed}}
+  (void)static_cast<C1*>((A*)0); // expected-error {{cannot cast 'struct A *' to 'struct C1 *' via virtual base 'struct B'}}
+  (void)static_cast<C1&>(*((A*)0)); // expected-error {{cannot cast 'struct A' to 'struct C1 &' via virtual base 'struct B'}}
+  (void)static_cast<D*>((A*)0); // expected-error {{cannot cast 'struct A *' to 'struct D *' via virtual base 'struct B'}}
+  (void)static_cast<D&>(*((A*)0)); // expected-error {{cannot cast 'struct A' to 'struct D &' via virtual base 'struct B'}}
+  (void)static_cast<B*>((const A*)0); // expected-error {{static_cast from 'struct A const *' to 'struct B *' casts away constness}}
+  (void)static_cast<B&>(*((const A*)0)); // expected-error {{static_cast from 'struct A const' to 'struct B &' casts away constness}}
   // Accessibility is not yet tested
   //(void)static_cast<E*>((A*)0); // {{static_cast from 'struct A *' to 'struct E *' is not allowed}}
   //(void)static_cast<E&>(*((A*)0)); // {{static_cast from 'struct A' to 'struct E &' is not allowed}}
-  (void)static_cast<H*>((A*)0); // expected-error {{static_cast from 'struct A *' to 'struct H *' is not allowed}}
-  (void)static_cast<H&>(*((A*)0)); // expected-error {{static_cast from 'struct A' to 'struct H &' is not allowed}}
+  (void)static_cast<H*>((A*)0); // expected-error {{ambiguous static_cast from base 'struct A' to derived 'struct H':\n    struct A -> struct B -> struct G1 -> struct H\n    struct A -> struct B -> struct G2 -> struct H}}
+  (void)static_cast<H&>(*((A*)0)); // expected-error {{ambiguous static_cast from base 'struct A' to derived 'struct H':\n    struct A -> struct B -> struct G1 -> struct H\n    struct A -> struct B -> struct G2 -> struct H}}
   (void)static_cast<E*>((B*)0); // expected-error {{static_cast from 'struct B *' to 'struct E *' is not allowed}}
-  (void)static_cast<E&>(*((B*)0)); // expected-error {{static_cast from 'struct B' to 'struct E &' is not allowed}}
+  (void)static_cast<E&>(*((B*)0)); // expected-error {{non-const reference to type 'struct E' cannot be initialized with a value of type 'struct B'}}
 
   // TODO: Test inaccessible base in context where it's accessible, i.e.
   // member function and friend.





More information about the cfe-commits mailing list