WIP: Fix gcc, clang and msvc warnings#607
Open
skwerner wants to merge 19 commits into
Open
Conversation
- Add explicit copy constructors to BBox, LBBox, CubicBezierCurve - Add explicit copy assignment operator to range - Fix base class initialization in Atomic copy constructor - Suppress -Wunused-parameter, -Wcast-function-type, -Wdeprecated-copy
stefanatwork
changed the title
- Fix viewer.cpp: wrap extern C variables with initializers in extern C blocks - Fix viewer.cpp: remove redundant renderFrameStandard declaration - Fix verify.cpp: add [[fallthrough]] for case 20 fall-through - Fix verify.cpp: remove extra semicolon after ALIGNED_CLASS_ macro - Fix constraints.h: remove extra semicolon after ALIGNED_CLASS_ macro - Fix intersection_filter_device.cpp: construct Ray directly instead of default-init + assign - Fix clang.cmake: add -Wno-c++17-attribute-extensions for [[fallthrough]] in C++11 mode Both GCC-15 and Clang-21 builds with tutorials now emit zero warnings/errors.
- Add -Wall -Wextra -Werror with appropriate suppressions to dpcpp.cmake - Remove unused hasDriverExtension function and hDriver variable - Fix const return types on uint32_t/bool functions - Fix sign comparison in instance_array.h and thread.cpp
Add /W4 (high warning level) and /WX (warnings as errors) to the MSVC build. Suppress a curated set of warnings that fire on intentional patterns throughout the codebase: /wd4100 - unreferenced formal parameter (template/virtual boilerplate) /wd4127 - constant conditional expression (template/SIMD idioms) /wd4201 - nameless struct/union (SIMD vector types) /wd4244 - narrowing conversion (geometry/SIMD math) /wd4267 - size_t narrowing (index arithmetic) /wd4324 - struct padded for alignment (SIMD alignment specifiers) /wd4512 - assignment operator not generated (const-member types) /wd4714 - __forceinline not inlined (compiler decision) /wd4702 - unreachable code (false positives in inlined templates) /wd4800 - implicit int-to-bool (flag/mask code) Also strip the CMake-injected /W3 so /W4 is unambiguous. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve all MSVC warnings that fire under /W4 /WX in the core library
(common/) and BVH kernels (kernels/). Changes are purely mechanical
renames and casts — no logic is altered.
Warning categories fixed:
C4189 unused initialized local variable (alloc.h: 'done' only live
inside assert, which is a no-op in Release; add (void)done)
C4245 signed/unsigned mismatch (ray.mask = -1 -> 0xFFFFFFFFu, and
various index/count assignments)
C4456 local declaration hides outer local (loop-variable shadowing)
C4457 local declaration hides function parameter
C4458 local declaration hides class member (lambda captures, etc.)
C4459 local declaration hides global (parameter named same as global)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolve all MSVC warnings under /W4 /WX across every tutorial and the
shared tutorial infrastructure.
Warning categories fixed:
C4189 unused initialized local (alloc.h 'done' pattern)
C4245 signed/unsigned mismatch — ray/shadow mask initialisation
(mask = -1 -> 0xFFFFFFFFu) and size_t argument mismatches
C4389 signed/unsigned mismatch in == operator
C4456 local hides outer local — shadowed loop variables (i, j),
mesh, hgeom, vertices, quad, tri
C4457 local hides function parameter (v in geometry lambdas)
C4458 local hides class member (N, ivariant, sampler)
C4459 local hides global — renderPixelStandard/postIntersect/
createPoints 'data' parameter renamed to 'td' throughout;
quaternion_motion_blur 'qdc' parameter renamed
C4505 unreferenced static function — benchmark.h helpers changed
from 'static MAYBE_UNUSED' to 'inline'
C5051 [[maybe_unused]] / [[fallthrough]] require /std:c++17;
replaced with MAYBE_UNUSED macro and /* falls through */
comments respectively
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The SYCL_ONEAPI_ICX detection checked for 'icx' and 'icpx' but not for 'icx-cl', the MSVC-compatibility-mode variant of the ICX compiler used when building with Ninja/CMake on Windows. Without this match the cmake logic fell through to the Linux/Clang flag path, injecting flags such as -std=c++17, -fvisibility=hidden and -ffp-model=precise that icx-cl rejects with -Werror,-Wunknown-argument errors. Add 'icx-cl' to the SYCL_ONEAPI_ICX name check so the correct Windows ICX flag set is selected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Three classes of warning fixed, all from the ICX SYCL device compiler:
1. [-Wundefined-internal] sycl::detail::get_spec_constant_symbolic_ID
has internal linkage but is not defined.
'const' variables at C++ namespace scope have *internal* linkage by
default (same as 'static const'). The SYCL compiler generates a
per-variable helper function with matching linkage; when that linkage
is internal the function has no definition visible to the SYCL
runtime and the warning fires.
Fix: declare all sycl::specialization_id objects as 'inline const'.
C++17 inline variables have external linkage, so the compiler emits
one uniquely-identifiable definition. viewer_device_debug.cpp
re-declares the same spec_feature_mask as inline const; the ODR
rule merges it with viewer_device.cpp's definition at link time.
Files changed: viewer_device.cpp, viewer_device_debug.cpp,
next_hit_device.cpp, pathtracer_device.cpp
2. [-Wuninitialized-const-pointer] imgui_widgets.cpp: 'empty_string'
passed as a const-pointer before being initialised.
Fix: zero-initialise the STB_TEXTEDIT_CHARTYPE variable at the point
of declaration.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
stefanatwork
force-pushed
the
fix-all-warnings
branch
from
f4cb0f9 to
7376304
Compare
Use (int)(1u << 31) instead of 1 << 31 to avoid -Werror=shift-overflow= when shifting into the sign bit of a signed int. Keeps the constant type consistent with all other CPU_FEATURE_BIT_* constants. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The PR renamed locals v0/v1/v2 to lv0/lv1/lv2 to fix shadowing warnings, but forgot to update the store_nt() call in fill(), causing it to pass the stale member vectors instead of the newly loaded triangle vertices. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
update() declared locals lv0/lv1/lv2 but used the member v3 directly, leaving stale data in unused lanes and being inconsistent with the other vertex vectors. Add a zero-initialized local lv3 and pass it into the placement-new, matching the pattern used by fill() and by trianglev.h. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
… Clang The previous change stopped applying FLAGS_LOWEST on MSVC and passed -Wno-uninitialized-const-pointer to all non-MSVC compilers including GCC, which may not support that flag. Restore the unconditional FLAGS_LOWEST (matching master) and gate the Clang-specific warning suppression behind a Clang compiler check. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comparing ray.instID[0] (unsigned) to -1 triggers -Wsign-compare. Use RTC_INVALID_GEOMETRY_ID consistently, matching the check on line 775. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
'const' variables at namespace scope have internal linkage by default. The SYCL compiler generates get_spec_constant_symbolic_ID<feature_mask> with matching linkage, causing [-Wundefined-internal] when -Werror is set. Fix: declare feature_mask as 'inline const', giving it external linkage so the compiler emits a single visible definition. Same fix already applied to viewer_device.cpp, viewer_device_debug.cpp, next_hit_device.cpp, and pathtracer_device.cpp in eecccf3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The SYCL compiler runs a separate device-side compilation pass that re-includes headers without any of the host-side pragma guards. sycl/access/access.hpp uses constant_space internally and warns about its own deprecated usage during this pass. The pragma in tutorials/common/device_default.h already suppresses this for the host pass when including sycl.hpp, but it has no effect on the device pass. Fix by adding -Wno-deprecated-declarations to CMAKE_CXX_FLAGS_SYCL so the suppression applies to both passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace -I with -imsvc for Intel oneAPI SYCL include directories on Windows (icx-cl), matching the -isystem flags already used on Linux. This suppresses deprecation warnings emitted by Intel's own SYCL runtime headers (e.g. constant_space in sycl/access/access.hpp) without blanket-suppressing -Wdeprecated-declarations in Embree code. Remove the previously added -Wno-deprecated-declarations workaround. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
svenwoop
requested changes
Jun 18, 2026
| __forceinline BBox ( ) { } | ||
| template<typename T1> | ||
| __forceinline BBox ( const BBox<T1>& other ) : lower(other.lower), upper(other.upper) {} | ||
| BBox ( const BBox& other ) = default; |
| __forceinline LBBox ( const LBBox<T1>& other ) | ||
| : bounds0(other.bounds0), bounds1(other.bounds1) {} | ||
|
|
||
| LBBox ( const LBBox& other ) = default; |
| numUserThreads = 0; | ||
|
|
||
| #if TASKING_INTERNAL | ||
| #ifdef TASKING_INTERNAL |
Collaborator
There was a problem hiding this comment.
Hm, this changes behavior now, looks like this was before never setting affinity. Shouldn't we just always set affinity to false? Some people are using internal tasking, and setting affinity now could impact them. Please double check if this was really false for internal tasking system before.
| : v0(other.v0), v1(other.v1), v2(other.v2), v3(other.v3) {} | ||
|
|
||
|
|
||
| CubicBezierCurve (const CubicBezierCurve& other) = default; |
| } | ||
|
|
||
| __forceinline void eval(const float u, const float v, | ||
| __forceinline void eval(const float u, const float vv, |
Collaborator
There was a problem hiding this comment.
consistenly name first argument uu
| if (mesh->normals.size()) mesh->normals.resize(1); | ||
| if (mesh->tangents.size()) mesh->tangents.resize(1); | ||
| if (mesh->dnormals.size()) mesh->dnormals.resize(1); | ||
| else if (Ref<SceneGraph::HairSetNode> mesh3 = node.dynamicCast<SceneGraph::HairSetNode>()) { |
Collaborator
There was a problem hiding this comment.
which compiler is complaining here? renaming all these makes not much sense, the variables are all introduced in a different if statement.
| store(light->light,id); | ||
| else if (auto light = node.dynamicCast<SceneGraph::LightNodeImpl<SceneGraph::QuadLight>>()) | ||
| store(light->light,id); | ||
| if (auto lightA = node.dynamicCast<SceneGraph::LightNodeImpl<SceneGraph::AmbientLight>>()) |
Collaborator
There was a problem hiding this comment.
again, this makes the code not better but worse
| } | ||
|
|
||
| Vec3fa noise3D(const Vec3fa& p) | ||
| Vec3fa noise3D(const Vec3fa& pos) |
Collaborator
There was a problem hiding this comment.
why is this renamed, does this cause a warning, why?
| TutorialData data; | ||
|
|
||
| Vec3fa interpolate_linear(const TutorialData& data, unsigned int primID, float u) | ||
| Vec3fa interpolate_linear(const TutorialData& tutorialData, unsigned int primID, float u) |
Collaborator
There was a problem hiding this comment.
these scoping renames make no sense, and I do not think we should do this. This here means I never can call a function argument data, because there is also a global variable named data!?
| { | ||
| const Vec3fa c0 = ((Vec3fa*) data.hair_vertex_colors)[primID+1]; | ||
| const Vec3fa c1 = ((Vec3fa*) data.hair_vertex_colors)[primID+2]; | ||
| const Vec3fa c0 = ((Vec3fa*) tutorialData.hair_vertex_colors)[primID+1]; |
Collaborator
There was a problem hiding this comment.
Also we should keep ISPC and CPP version in sync, if, then these changes should also be done to ISPC version.
stefanatwork
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.