Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Lint pass: ctor initializer order + virtual dtors #865

Merged
merged 3 commits into from
Feb 22, 2021
Merged

Lint pass: ctor initializer order + virtual dtors #865

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba94da1
Lint pass: ctor initializer order + virtual dtors
hollasch Feb 14, 2021
2c29520
Added warnings for clang and gnu to CMake
trevordblack Feb 22, 2021
944fd9b
Cleaned up warnings found in clang and gnu
trevordblack Feb 22, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Change Log -- Ray Tracing in One Weekend
- Change: `hittable` member variable `ptr` renamed to `object`
- Change: general rename of `mat_ptr` to `mat` (material)
- Change: hittable::bounding_box() signature has changed to always return a value (#859)
- Fix: Enabled compiler warnings for MSVC, Clang, GNU. Cleaned up warnings as fit (#865)

### In One Weekend
- Added: More commentary about the choice between `double` and `float` (#752)
Expand Down
26 changes: 24 additions & 2 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ set ( CMAKE_CXX_EXTENSIONS OFF )
set ( COMMON_ALL
src/common/rtweekend.h
src/common/camera.h
src/common/color.h
src/common/interval.h
src/common/ray.h
src/common/vec3.h
)
Expand Down Expand Up @@ -71,6 +73,28 @@ set ( SOURCE_REST_OF_YOUR_LIFE
src/TheRestOfYourLife/main.cc
)

include_directories(src/common)

# Specific Compiler Flags

message (STATUS "Compiler ID: " ${CMAKE_CXX_COMPILER_ID})

if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
add_compile_options("/we 4265") # Class has virtual functions, but its non-trivial destructor is not virtual
add_compile_options("/w3 5038") # Data member will be initialized after [other] data member
add_compile_options("/we 5204") # Class has virtual functions, but its trivial destructor is not virtual
elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
add_compile_options(-Wnon-virtual-dtor) # Class has virtual functions, but its destructor is not virtual
add_compile_options(-Wreorder) # Data member will be initialized after [other] data member
add_compile_options(-Wmaybe-uninitialized) # Variable improperly initialized
add_compile_options(-Wunused-variable) # Variable is defined but unused
elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options(-Wnon-virtual-dtor) # Class has virtual functions, but its destructor is not virtual
add_compile_options(-Wreorder) # Data member will be initialized after [other] data member
add_compile_options(-Wsometimes-uninitialized) # Variable improperly initialized
add_compile_options(-Wunused-variable) # Variable is defined but unused
endif()

# Executables
add_executable(inOneWeekend ${SOURCE_ONE_WEEKEND})
add_executable(theNextWeek ${SOURCE_NEXT_WEEK})
Expand All @@ -82,5 +106,3 @@ add_executable(pi src/TheRestOfYourLife/pi.cc ${CO
add_executable(estimate_halfway src/TheRestOfYourLife/estimate_halfway.cc ${COMMON_ALL})
add_executable(sphere_importance src/TheRestOfYourLife/sphere_importance.cc ${COMMON_ALL})
add_executable(sphere_plot src/TheRestOfYourLife/sphere_plot.cc ${COMMON_ALL})

include_directories(src/common)
4 changes: 4 additions & 0 deletions books/RayTracingInOneWeekend.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,8 @@

class hittable {
public:
virtual ~hittable() = default;

virtual bool hit(const ray& r, double ray_tmin, double ray_tmax, hit_record& rec) const = 0;
};

Expand Down Expand Up @@ -2033,6 +2035,8 @@

class material {
public:
virtual ~material() = default;

virtual bool scatter(
const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered) const = 0;
};
Expand Down
17 changes: 11 additions & 6 deletions books/RayTracingTheNextWeek.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1099,6 +1099,8 @@

class texture {
public:
virtual ~texture() = default;

virtual color value(double u, double v, const point3& p) const = 0;
};

Expand Down Expand Up @@ -1194,8 +1196,8 @@

public:
double inv_scale;
shared_ptr<texture> odd;
shared_ptr<texture> even;
shared_ptr<texture> odd;
};
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[Listing [checker-texture]: <kbd>[texture.h]</kbd> Checkered texture]
Expand Down Expand Up @@ -2293,12 +2295,15 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
class material {
public:
...


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
virtual color emitted(double u, double v, const point3& p) const {
return color(0,0,0);
}

~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++

virtual bool scatter(
const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered
) const = 0;
Expand Down Expand Up @@ -2466,8 +2471,8 @@
aabb bounding_box() const override { return bbox; }

public:
shared_ptr<material> mat;
double x0, x1, y0, y1, k;
shared_ptr<material> mat;
aabb bbox;
};

Expand Down Expand Up @@ -2590,8 +2595,8 @@
aabb bounding_box() const override { return bbox; }

public:
shared_ptr<material> mat;
double x0, x1, z0, z1, k;
shared_ptr<material> mat;
aabb bbox;
};

Expand Down Expand Up @@ -2625,8 +2630,8 @@
aabb bounding_box() const override { return bbox; }

public:
shared_ptr<material> mat;
double y0, y1, z0, z1, k;
shared_ptr<material> mat;
aabb bbox;
};
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down Expand Up @@ -3120,8 +3125,8 @@

public:
shared_ptr<hittable> boundary;
shared_ptr<material> phase_function;
double neg_inv_density;
shared_ptr<material> phase_function;
};

#endif
Expand Down
37 changes: 13 additions & 24 deletions books/RayTracingTheRestOfYourLife.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@
}
}

auto N = static_cast<double>(sqrt_N) * sqrt_N;
std::cout << std::fixed << std::setprecision(12);
std::cout
<< "Regular Estimate of Pi = "
Expand Down Expand Up @@ -886,7 +885,7 @@

// Find out the sample at which we have half of our area
double half_sum = sum / 2.0;
double halfway_point;
double halfway_point = 0.0;
double accum = 0.0;
for (int i = 0; i < N; i++){
accum += samples[i].p_x;
Expand Down Expand Up @@ -1504,9 +1503,7 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
class material {
public:
virtual color emitted(double u, double v, const point3& p) const {
return color(0,0,0);
}
...


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
Expand Down Expand Up @@ -2305,14 +2302,16 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
class material {
public:
...


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
virtual color emitted(
const ray& r_in, const hit_record& rec, double u, double v, const point3& p
) const {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
return color(0,0,0);
}

...
};

Expand Down Expand Up @@ -2523,21 +2522,21 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
class hittable_pdf : public pdf {
public:
hittable_pdf(const hittable_list& _hittable_ptr, const point3& _origin)
: hittable_ptr(_hittable_ptr), origin(_origin)
hittable_pdf(const hittable_list& _objects, const point3& _origin)
: objects(_objects), origin(_origin)
{}

double value(const vec3& direction) const override {
return hittable_ptr->pdf_value(origin, direction);
return objects.pdf_value(origin, direction);
}

vec3 generate() const override {
return hittable_ptr->random(origin);
return objects.random(origin);
}

public:
const hittable_list& objects;
point3 origin;
shared_ptr<hittable> hittable_ptr;
};
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[Listing [class-hittable-pdf]: <kbd>[pdf.h]</kbd> The hittable_pdf class]
Expand All @@ -2555,9 +2554,7 @@
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
class hittable {
public:
virtual bool hit(const ray& r, interval ray_t, hit_record& rec) const = 0;

virtual aabb bounding_box() const = 0;
...


~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
Expand Down Expand Up @@ -2890,11 +2887,7 @@

class material {
public:
virtual color emitted(
const ray& r_in, const hit_record& rec, double u, double v, const point3& p
) const {
return color(0,0,0);
}
...

virtual bool scatter(
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
Expand All @@ -2903,11 +2896,7 @@
) const {
return false;
}

virtual double scattering_pdf(const ray& r_in, const hit_record& rec, const ray& scattered)
const {
return 0;
}
...
};
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[Listing [material-refactor]: <kbd>[material.h]</kbd> Refactoring the material class]
Expand Down
2 changes: 2 additions & 0 deletions src/InOneWeekend/hittable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ class hit_record {

class hittable {
public:
virtual ~hittable() = default;

virtual bool hit(const ray& r, interval ray_t, hit_record& rec) const = 0;
};

Expand Down
2 changes: 2 additions & 0 deletions src/InOneWeekend/material.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ class hit_record;

class material {
public:
virtual ~material() = default;

virtual bool scatter(
const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered
) const = 0;
Expand Down
6 changes: 3 additions & 3 deletions src/TheNextWeek/aarect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ class xy_rect : public hittable {
aabb bounding_box() const override { return bbox; }

public:
shared_ptr<material> mat;
double x0, x1, y0, y1, k;
shared_ptr<material> mat;
aabb bbox;
};

Expand Down Expand Up @@ -88,8 +88,8 @@ class xz_rect : public hittable {
aabb bounding_box() const override { return bbox; }

public:
shared_ptr<material> mat;
double x0, x1, z0, z1, k;
shared_ptr<material> mat;
aabb bbox;
};

Expand Down Expand Up @@ -127,8 +127,8 @@ class yz_rect : public hittable {
aabb bounding_box() const override { return bbox; }

public:
shared_ptr<material> mat;
double y0, y1, z0, z1, k;
shared_ptr<material> mat;
aabb bbox;
};

Expand Down
2 changes: 1 addition & 1 deletion src/TheNextWeek/constant_medium.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ class constant_medium : public hittable {

public:
shared_ptr<hittable> boundary;
shared_ptr<material> phase_function;
double neg_inv_density;
shared_ptr<material> phase_function;
};


Expand Down
2 changes: 2 additions & 0 deletions src/TheNextWeek/hittable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class hit_record {

class hittable {
public:
virtual ~hittable() = default;

virtual bool hit(const ray& r, interval ray_t, hit_record& rec) const = 0;

virtual aabb bounding_box() const = 0;
Expand Down
2 changes: 2 additions & 0 deletions src/TheNextWeek/material.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

class material {
public:
virtual ~material() = default;

virtual color emitted(double u, double v, const point3& p) const {
return color(0,0,0);
}
Expand Down
6 changes: 3 additions & 3 deletions src/TheRestOfYourLife/aarect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ class xy_rect : public hittable {
aabb bounding_box() const override { return bbox; }

public:
shared_ptr<material> mat;
double x0, x1, y0, y1, k;
shared_ptr<material> mat;
aabb bbox;
};

Expand Down Expand Up @@ -105,8 +105,8 @@ class xz_rect : public hittable {
}

public:
shared_ptr<material> mat;
double x0, x1, z0, z1, k;
shared_ptr<material> mat;
aabb bbox;
};

Expand Down Expand Up @@ -143,8 +143,8 @@ class yz_rect : public hittable {

aabb bounding_box() const override { return bbox; }
public:
shared_ptr<material> mat;
double y0, y1, z0, z1, k;
shared_ptr<material> mat;
aabb bbox;
};

Expand Down
2 changes: 1 addition & 1 deletion src/TheRestOfYourLife/estimate_halfway.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ int main() {

// Find out the sample at which we have half of our area
double half_sum = sum / 2.0;
double halfway_point;
double halfway_point = 0.0;
double accum = 0.0;
for (int i = 0; i < N; i++){
accum += samples[i].p_x;
Expand Down
2 changes: 2 additions & 0 deletions src/TheRestOfYourLife/hittable.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class hit_record {

class hittable {
public:
virtual ~hittable() = default;

virtual bool hit(const ray& r, interval ray_t, hit_record& rec) const = 0;

virtual aabb bounding_box() const = 0;
Expand Down
2 changes: 0 additions & 2 deletions src/TheRestOfYourLife/integrate_x_sq.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@ double pdf(double x) {
}

int main() {
int inside_circle = 0;
int inside_circle_stratified = 0;
int N = 1;

auto sum = 0.0;
Expand Down
2 changes: 2 additions & 0 deletions src/TheRestOfYourLife/material.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ class scatter_record {

class material {
public:
virtual ~material() = default;

virtual color emitted(
const ray& r_in, const hit_record& rec, double u, double v, const point3& p
) const {
Expand Down
Loading