From b6132db5390bfa513aa8a46f162d8d9cb8634961 Mon Sep 17 00:00:00 2001 From: Shalom Craimer Date: Thu, 4 May 2017 10:13:22 +0300 Subject: [PATCH 1/4] C.148 adding Reason and Example --- CppCoreGuidelines.md | 48 +++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 45 insertions(+), 3 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index e0fdf3d..3e7df8c 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -7296,15 +7296,57 @@ Casting to a reference expresses that you intend to end up with a valid object, ##### Reason -??? +A failure to find the required class will cause `dynamic_cast` to return a null value, and de-referencing a null-valued pointer will lead to undefined behaviour. +Therefore the result of the `dynamic_cast` should always be treated as if it may contain a null value, and tested. + +If such a failure is contrary to your design, it should be an error; See [C.147](#Rh-ptr-cast). ##### Example - ??? +The example below describes a `ShapeOwner` that takes ownership of constructed `Shape` objects. The objects are also sorted into views, according to their geometric attributes. + + #include + #include + + struct GeometricAttribute { virtual ~GeometricAttribute() { } }; + + class TrilaterallySymmetrical : public GeometricAttribute { }; + class EvenSided : public GeometricAttribute { }; + + struct Shape { virtual ~Shape() { } }; + + class Triangle : public Shape, public TrilaterallySymmetrical { }; + class Square : public Shape, public EvenSided { }; + class Hexagon : public Shape, public EvenSided, public TrilaterallySymmetrical { }; + + class ShapeOwner { + std::vector> owned; + + std::vector view_of_evens; + std::vector view_of_trisyms; + + void add( Shape * const item ) + { + // Ownership is always taken + owned.emplace_back( item ); + + // Check the GeometricAttributes and add the shape to none/one/some/all of the views + + if( auto even = dynamic_cast( item ) ) + { + view_of_evens.emplace_back( even ); + } + + if( auto trisym = dynamic_cast( item ) ) + { + view_of_trisyms.emplace_back( trisym ); + } + } + }; ##### Enforcement -??? +* (Complex) Unless there is a null test on the result of a `dynamic_cast` of a pointer type, warn upon dereference of the pointer. ### C.149: Use `unique_ptr` or `shared_ptr` to avoid forgetting to `delete` objects created using `new` From 1c53b29a3af4c15fe82adf1014ec32c1ab564ef4 Mon Sep 17 00:00:00 2001 From: Shalom Craimer Date: Thu, 4 May 2017 11:19:31 +0300 Subject: [PATCH 2/4] C.148 - Fixing Travis-reported errors discovered so far in the code example --- CppCoreGuidelines.md | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 3e7df8c..a3daabf 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -7325,21 +7325,21 @@ The example below describes a `ShapeOwner` that takes ownership of constructed ` std::vector view_of_evens; std::vector view_of_trisyms; - void add( Shape * const item ) + void add(Shape * const item) { // Ownership is always taken - owned.emplace_back( item ); + owned.emplace_back(item); // Check the GeometricAttributes and add the shape to none/one/some/all of the views - if( auto even = dynamic_cast( item ) ) + if (auto even = dynamic_cast(item)) { - view_of_evens.emplace_back( even ); + view_of_evens.emplace_back(even); } - if( auto trisym = dynamic_cast( item ) ) + if (auto trisym = dynamic_cast(item)) { - view_of_trisyms.emplace_back( trisym ); + view_of_trisyms.emplace_back(trisym); } } }; From cdf2e7e5ea8e33558f798d064779dfae2b18c777 Mon Sep 17 00:00:00 2001 From: Shalom Craimer Date: Fri, 5 May 2017 00:56:29 +0300 Subject: [PATCH 3/4] Fixed the errors detected by Travis CI and @jwakely --- CppCoreGuidelines.md | 60 ++++++++++++++++---------------------------- 1 file changed, 22 insertions(+), 38 deletions(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index a3daabf..14463d3 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -7296,53 +7296,37 @@ Casting to a reference expresses that you intend to end up with a valid object, ##### Reason -A failure to find the required class will cause `dynamic_cast` to return a null value, and de-referencing a null-valued pointer will lead to undefined behaviour. -Therefore the result of the `dynamic_cast` should always be treated as if it may contain a null value, and tested. +The `dynamic_cast` conversion allows to test whether a pointer is pointing at a polymorphic object that has a given class in its hierarchy. Since failure to find the class merely returns a null value, it can be tested during run-time. This allows writing code that can choose alternative paths depending on the results. -If such a failure is contrary to your design, it should be an error; See [C.147](#Rh-ptr-cast). +Contrast with [C.147](#Rh-ptr-cast), where failure is an error, and should not be used for conditional execution. ##### Example -The example below describes a `ShapeOwner` that takes ownership of constructed `Shape` objects. The objects are also sorted into views, according to their geometric attributes. +The example below describes a `Shape_owner` that takes ownership of constructed `Shape` objects. The objects are also sorted into views, according to their geometric attributes. +In this example, `Shape` does not inherit from `Geometric_attributes`. Only its subclasses do. - #include - #include + void add(Shape* const item) + { + // Ownership is always taken + owned_shapes.emplace_back(item); - struct GeometricAttribute { virtual ~GeometricAttribute() { } }; + // Check the Geometric_attributes and add the shape to none/one/some/all of the views - class TrilaterallySymmetrical : public GeometricAttribute { }; - class EvenSided : public GeometricAttribute { }; - - struct Shape { virtual ~Shape() { } }; - - class Triangle : public Shape, public TrilaterallySymmetrical { }; - class Square : public Shape, public EvenSided { }; - class Hexagon : public Shape, public EvenSided, public TrilaterallySymmetrical { }; - - class ShapeOwner { - std::vector> owned; - - std::vector view_of_evens; - std::vector view_of_trisyms; - - void add(Shape * const item) + if (auto even = dynamic_cast(item)) { - // Ownership is always taken - owned.emplace_back(item); - - // Check the GeometricAttributes and add the shape to none/one/some/all of the views - - if (auto even = dynamic_cast(item)) - { - view_of_evens.emplace_back(even); - } - - if (auto trisym = dynamic_cast(item)) - { - view_of_trisyms.emplace_back(trisym); - } + view_of_evens.emplace_back(even); } - }; + + if (auto trisym = dynamic_cast(item)) + { + view_of_trisyms.emplace_back(trisym); + } + } + +##### Notes + +A failure to find the required class will cause `dynamic_cast` to return a null value, and de-referencing a null-valued pointer will lead to undefined behavior. +Therefore the result of the `dynamic_cast` should always be treated as if it may contain a null value, and tested. ##### Enforcement From 12f0954f663a4935f441fd6289085df32ed19805 Mon Sep 17 00:00:00 2001 From: Shalom Craimer Date: Fri, 5 May 2017 01:20:16 +0300 Subject: [PATCH 4/4] Fixed the errors detected by Travis CI and @jwakely --- CppCoreGuidelines.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CppCoreGuidelines.md b/CppCoreGuidelines.md index 14463d3..984bd09 100644 --- a/CppCoreGuidelines.md +++ b/CppCoreGuidelines.md @@ -7302,7 +7302,7 @@ Contrast with [C.147](#Rh-ptr-cast), where failure is an error, and should not b ##### Example -The example below describes a `Shape_owner` that takes ownership of constructed `Shape` objects. The objects are also sorted into views, according to their geometric attributes. +The example below describes the `add` method of a `Shape_owner` that takes ownership of constructed `Shape` objects. The objects are also sorted into views, according to their geometric attributes. In this example, `Shape` does not inherit from `Geometric_attributes`. Only its subclasses do. void add(Shape* const item)