Skip to content

Commit 9540eea

Browse files
hollaschhollasch
authored
Merge pull request #865 from RayTracing/lint-pass
Lint pass: ctor initializer order + virtual dtors
2 parents 94fa208 + 944fd9b commit 9540eea

19 files changed

+81
-49
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ Change Log -- Ray Tracing in One Weekend
2121
- Change: `hittable` member variable `ptr` renamed to `object`
2222
- Change: general rename of `mat_ptr` to `mat` (material)
2323
- Change: hittable::bounding_box() signature has changed to always return a value (#859)
24+
- Fix: Enabled compiler warnings for MSVC, Clang, GNU. Cleaned up warnings as fit (#865)
2425

2526
### In One Weekend
2627
- Added: More commentary about the choice between `double` and `float` (#752)

‎CMakeLists.txt

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ set ( CMAKE_CXX_EXTENSIONS OFF )
1717
set ( COMMON_ALL
1818
src/common/rtweekend.h
1919
src/common/camera.h
20+
src/common/color.h
21+
src/common/interval.h
2022
src/common/ray.h
2123
src/common/vec3.h
2224
)
@@ -71,6 +73,28 @@ set ( SOURCE_REST_OF_YOUR_LIFE
7173
src/TheRestOfYourLife/main.cc
7274
)
7375

76+
include_directories(src/common)
77+
78+
# Specific Compiler Flags
79+
80+
message (STATUS "Compiler ID: " ${CMAKE_CXX_COMPILER_ID})
81+
82+
if (CMAKE_CXX_COMPILER_ID STREQUAL "MSVC")
83+
add_compile_options("/we 4265") # Class has virtual functions, but its non-trivial destructor is not virtual
84+
add_compile_options("/w3 5038") # Data member will be initialized after [other] data member
85+
add_compile_options("/we 5204") # Class has virtual functions, but its trivial destructor is not virtual
86+
elseif (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
87+
add_compile_options(-Wnon-virtual-dtor) # Class has virtual functions, but its destructor is not virtual
88+
add_compile_options(-Wreorder) # Data member will be initialized after [other] data member
89+
add_compile_options(-Wmaybe-uninitialized) # Variable improperly initialized
90+
add_compile_options(-Wunused-variable) # Variable is defined but unused
91+
elseif (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
92+
add_compile_options(-Wnon-virtual-dtor) # Class has virtual functions, but its destructor is not virtual
93+
add_compile_options(-Wreorder) # Data member will be initialized after [other] data member
94+
add_compile_options(-Wsometimes-uninitialized) # Variable improperly initialized
95+
add_compile_options(-Wunused-variable) # Variable is defined but unused
96+
endif()
97+
7498
# Executables
7599
add_executable(inOneWeekend ${SOURCE_ONE_WEEKEND})
76100
add_executable(theNextWeek ${SOURCE_NEXT_WEEK})
@@ -82,5 +106,3 @@ add_executable(pi src/TheRestOfYourLife/pi.cc ${CO
82106
add_executable(estimate_halfway src/TheRestOfYourLife/estimate_halfway.cc ${COMMON_ALL})
83107
add_executable(sphere_importance src/TheRestOfYourLife/sphere_importance.cc ${COMMON_ALL})
84108
add_executable(sphere_plot src/TheRestOfYourLife/sphere_plot.cc ${COMMON_ALL})
85-
86-
include_directories(src/common)

‎books/RayTracingInOneWeekend.html

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,8 @@
867867

868868
class hittable {
869869
public:
870+
virtual ~hittable() = default;
871+
870872
virtual bool hit(const ray& r, double ray_tmin, double ray_tmax, hit_record& rec) const = 0;
871873
};
872874

@@ -2033,6 +2035,8 @@
20332035

20342036
class material {
20352037
public:
2038+
virtual ~material() = default;
2039+
20362040
virtual bool scatter(
20372041
const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered) const = 0;
20382042
};

‎books/RayTracingTheNextWeek.html

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1099,6 +1099,8 @@
10991099

11001100
class texture {
11011101
public:
1102+
virtual ~texture() = default;
1103+
11021104
virtual color value(double u, double v, const point3& p) const = 0;
11031105
};
11041106

@@ -1194,8 +1196,8 @@
11941196

11951197
public:
11961198
double inv_scale;
1197-
shared_ptr<texture> odd;
11981199
shared_ptr<texture> even;
1200+
shared_ptr<texture> odd;
11991201
};
12001202
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
12011203
[Listing [checker-texture]: <kbd>[texture.h]</kbd> Checkered texture]
@@ -2293,12 +2295,15 @@
22932295
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
22942296
class material {
22952297
public:
2298+
...
2299+
2300+
22962301
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
22972302
virtual color emitted(double u, double v, const point3& p) const {
22982303
return color(0,0,0);
22992304
}
2300-
23012305
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
2306+
23022307
virtual bool scatter(
23032308
const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered
23042309
) const = 0;
@@ -2466,8 +2471,8 @@
24662471
aabb bounding_box() const override { return bbox; }
24672472

24682473
public:
2469-
shared_ptr<material> mat;
24702474
double x0, x1, y0, y1, k;
2475+
shared_ptr<material> mat;
24712476
aabb bbox;
24722477
};
24732478

@@ -2590,8 +2595,8 @@
25902595
aabb bounding_box() const override { return bbox; }
25912596

25922597
public:
2593-
shared_ptr<material> mat;
25942598
double x0, x1, z0, z1, k;
2599+
shared_ptr<material> mat;
25952600
aabb bbox;
25962601
};
25972602

@@ -2625,8 +2630,8 @@
26252630
aabb bounding_box() const override { return bbox; }
26262631

26272632
public:
2628-
shared_ptr<material> mat;
26292633
double y0, y1, z0, z1, k;
2634+
shared_ptr<material> mat;
26302635
aabb bbox;
26312636
};
26322637
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -3120,8 +3125,8 @@
31203125

31213126
public:
31223127
shared_ptr<hittable> boundary;
3123-
shared_ptr<material> phase_function;
31243128
double neg_inv_density;
3129+
shared_ptr<material> phase_function;
31253130
};
31263131

31273132
#endif

‎books/RayTracingTheRestOfYourLife.html

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -212,7 +212,6 @@
212212
}
213213
}
214214

215-
auto N = static_cast<double>(sqrt_N) * sqrt_N;
216215
std::cout << std::fixed << std::setprecision(12);
217216
std::cout
218217
<< "Regular Estimate of Pi = "
@@ -886,7 +885,7 @@
886885

887886
// Find out the sample at which we have half of our area
888887
double half_sum = sum / 2.0;
889-
double halfway_point;
888+
double halfway_point = 0.0;
890889
double accum = 0.0;
891890
for (int i = 0; i < N; i++){
892891
accum += samples[i].p_x;
@@ -1504,9 +1503,7 @@
15041503
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
15051504
class material {
15061505
public:
1507-
virtual color emitted(double u, double v, const point3& p) const {
1508-
return color(0,0,0);
1509-
}
1506+
...
15101507

15111508

15121509
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
@@ -2305,14 +2302,16 @@
23052302
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
23062303
class material {
23072304
public:
2305+
...
2306+
2307+
23082308
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
23092309
virtual color emitted(
23102310
const ray& r_in, const hit_record& rec, double u, double v, const point3& p
23112311
) const {
23122312
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
23132313
return color(0,0,0);
23142314
}
2315-
23162315
...
23172316
};
23182317

@@ -2523,21 +2522,21 @@
25232522
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
25242523
class hittable_pdf : public pdf {
25252524
public:
2526-
hittable_pdf(const hittable_list& _hittable_ptr, const point3& _origin)
2527-
: hittable_ptr(_hittable_ptr), origin(_origin)
2525+
hittable_pdf(const hittable_list& _objects, const point3& _origin)
2526+
: objects(_objects), origin(_origin)
25282527
{}
25292528

25302529
double value(const vec3& direction) const override {
2531-
return hittable_ptr->pdf_value(origin, direction);
2530+
return objects.pdf_value(origin, direction);
25322531
}
25332532

25342533
vec3 generate() const override {
2535-
return hittable_ptr->random(origin);
2534+
return objects.random(origin);
25362535
}
25372536

25382537
public:
2538+
const hittable_list& objects;
25392539
point3 origin;
2540-
shared_ptr<hittable> hittable_ptr;
25412540
};
25422541
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
25432542
[Listing [class-hittable-pdf]: <kbd>[pdf.h]</kbd> The hittable_pdf class]
@@ -2555,9 +2554,7 @@
25552554
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++
25562555
class hittable {
25572556
public:
2558-
virtual bool hit(const ray& r, interval ray_t, hit_record& rec) const = 0;
2559-
2560-
virtual aabb bounding_box() const = 0;
2557+
...
25612558

25622559

25632560
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
@@ -2890,11 +2887,7 @@
28902887

28912888
class material {
28922889
public:
2893-
virtual color emitted(
2894-
const ray& r_in, const hit_record& rec, double u, double v, const point3& p
2895-
) const {
2896-
return color(0,0,0);
2897-
}
2890+
...
28982891

28992892
virtual bool scatter(
29002893
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ C++ highlight
@@ -2903,11 +2896,7 @@
29032896
) const {
29042897
return false;
29052898
}
2906-
2907-
virtual double scattering_pdf(const ray& r_in, const hit_record& rec, const ray& scattered)
2908-
const {
2909-
return 0;
2910-
}
2899+
...
29112900
};
29122901
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
29132902
[Listing [material-refactor]: <kbd>[material.h]</kbd> Refactoring the material class]

‎src/InOneWeekend/hittable.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,8 @@ class hit_record {
3333

3434
class hittable {
3535
public:
36+
virtual ~hittable() = default;
37+
3638
virtual bool hit(const ray& r, interval ray_t, hit_record& rec) const = 0;
3739
};
3840

‎src/InOneWeekend/material.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ class hit_record;
1919

2020
class material {
2121
public:
22+
virtual ~material() = default;
23+
2224
virtual bool scatter(
2325
const ray& r_in, const hit_record& rec, color& attenuation, ray& scattered
2426
) const = 0;

‎src/TheNextWeek/aarect.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,8 @@ class xy_rect : public hittable {
4949
aabb bounding_box() const override { return bbox; }
5050

5151
public:
52-
shared_ptr<material> mat;
5352
double x0, x1, y0, y1, k;
53+
shared_ptr<material> mat;
5454
aabb bbox;
5555
};
5656

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

9090
public:
91-
shared_ptr<material> mat;
9291
double x0, x1, z0, z1, k;
92+
shared_ptr<material> mat;
9393
aabb bbox;
9494
};
9595

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

129129
public:
130-
shared_ptr<material> mat;
131130
double y0, y1, z0, z1, k;
131+
shared_ptr<material> mat;
132132
aabb bbox;
133133
};
134134

‎src/TheNextWeek/constant_medium.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,8 +79,8 @@ class constant_medium : public hittable {
7979

8080
public:
8181
shared_ptr<hittable> boundary;
82-
shared_ptr<material> phase_function;
8382
double neg_inv_density;
83+
shared_ptr<material> phase_function;
8484
};
8585

8686

‎src/TheNextWeek/hittable.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ class hit_record {
3838

3939
class hittable {
4040
public:
41+
virtual ~hittable() = default;
42+
4143
virtual bool hit(const ray& r, interval ray_t, hit_record& rec) const = 0;
4244

4345
virtual aabb bounding_box() const = 0;

0 commit comments

Comments
 (0)