Skip to content

Commit

Permalink
Fix new in clang 13.1 (Xcode13.3) warnings (KhronosGroup#553)
Browse files Browse the repository at this point in the history
These are set-but-not-used warnings new in Xcode 13.3.

* Rationalize suppressions of old and new warnings

* Incidentally, fix build_wasm_docker.sh to use EXIT trap to ensure terminating the docker container.
  • Loading branch information
MarkCallow authored Mar 25, 2022
1 parent ca1a81a commit fa3d0fd
Show file tree
Hide file tree
Showing 7 changed files with 61 additions and 42 deletions.
63 changes: 37 additions & 26 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -148,16 +148,6 @@ elseif(${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU"
add_compile_options(-Wall -Wextra)
add_compile_options( $<IF:$<CONFIG:Debug>,-O0,-O3> )
if(EMSCRIPTEN)
# For reasons I have yet to understand, the clang v12 used by the
# current Emscripten versions raises this deprecated copy warning on
# the assimp headers while the clang v12 used by Xcode does not. I am
# also unable to update the assimp headers as the macOS build of the
# matching version of libassimp fails. There has been no response
# from the assimp project and I don't have time to investigate.
# Therefore, since we don't have an Emscripten port of libassimp,
# meaning the relevant load tests will never be called, just globally
# suppress the warning.
add_compile_options(-Wno-deprecated-copy)
add_link_options( $<IF:$<CONFIG:Debug>,-gsource-map,-O3> )
else()
add_link_options( $<IF:$<CONFIG:Debug>,-g,-O3> )
Expand Down Expand Up @@ -489,6 +479,10 @@ PRIVATE
# Turn off these warnings until Rich fixes the occurences.
# It it not clear to me if generator expressions can be used here
# hence the long-winded way.
#message(STATUS
# "CMAKE_CXX_COMPILER_ID = ${CMAKE_CXX_COMPILER_ID} "
# "CMAKE_CXX_COMPILER_VERSION = ${CMAKE_CXX_COMPILER_VERSION}"
#)
if(MSVC)
# Currently no need to disable any warnings in basisu code. Rich fixed them.
elseif(${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU")
Expand All @@ -497,34 +491,51 @@ elseif(${CMAKE_CXX_COMPILER_ID} STREQUAL "GNU")
# disabled.
${BASISU_ENCODER_CXX_SRC}
PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-variable;-Wno-class-memaccess;-Wno-misleading-indentation;-Wno-extra;-Wno-deprecated-copy;-Wno-parentheses;-Wno-strict-aliasing"
)
)
set_source_files_properties(
lib/basisu/transcoder/basisu_transcoder.cpp
PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-function;-Wno-unused-variable;-Wno-class-memaccess;-Wno-maybe-uninitialized"
)
if (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "11.0")
set_source_files_properties(
lib/basisu/zstd/zstd.c
PROPERTIES COMPILE_OPTIONS "-Wno-unused-but-set-variable"
)
elseif(EMSCRIPTEN)
set_source_files_properties(
${BASISU_ENCODER_CXX_SRC}
PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-variable;-Wno-unused-parameter;-Wno-unused-but-set-variable"
)
set_source_files_properties(
lib/basisu/transcoder/basisu_transcoder.cpp
PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-function;-Wno-unused-variable"
)
set_source_files_properties(
lib/basisu/zstd/zstd.c
PROPERTIES COMPILE_OPTIONS "-Wno-unused-but-set-variable;-Wno-bitwise-instead-of-logical"
)
endif()
elseif(${CMAKE_CXX_COMPILER_ID} MATCHES "Clang")
#message(STATUS "CMAKE_CXX_COMPILER_VERSION = ${CMAKE_CXX_COMPILER_VERSION}")
if (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "12.0.5")
set_source_files_properties( lib/basisu/encoder/basisu_kernels_sse.cpp
PROPERTIES COMPILE_OPTIONS "-Wno-unused-parameter;-Wno-deprecated-copy;-Wno-uninitialized-const-reference"
)
else()
set_source_files_properties( lib/basisu/encoder/basisu_kernels_sse.cpp
PROPERTIES COMPILE_OPTIONS "-Wno-unused-parameter"
)
)
endif()
if (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "13.1")
set_source_files_properties(
# We haven't fixed zstd.c because the fix would have to be applied
# every time the upstream script is used to create an updated
# single file decoder.
lib/basisu/zstd/zstd.c
PROPERTIES COMPILE_OPTIONS "-Wno-unused-but-set-variable"
)
endif()
if (${CMAKE_CXX_COMPILER_VERSION} VERSION_GREATER_EQUAL "15.0")
# These are for Emscripten which is ahead of xcode in its clang
# version. Also future proofing for when xcode catches up.
set_source_files_properties(
${BASISU_ENCODER_CXX_SRC}
PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-variable;-Wno-unused-parameter"
)
set_source_files_properties(
lib/basisu/transcoder/basisu_transcoder.cpp
PROPERTIES COMPILE_OPTIONS "-Wno-sign-compare;-Wno-unused-function;-Wno-unused-variable"
)
set_source_files_properties(
lib/basisu/zstd/zstd.c
PROPERTIES COMPILE_OPTIONS "-Wno-unused-but-set-variable;-Wno-bitwise-instead-of-logical"
)
endif()
else()
message(ERROR "${CMAKE_CXX_COMPILER_ID} not yet supported.")
Expand Down
13 changes: 11 additions & 2 deletions lib/basisu/encoder/basisu_bc7enc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1281,6 +1281,15 @@ static uint64_t find_optimal_solution(uint32_t mode, bc7enc_vec4F xl, bc7enc_vec
return pResults->m_best_overall_err;
}

#if (__cplusplus >= 201703L)
#define MAYBE_UNUSED [[maybe_unused]]
#elif __GNUC__ || __clang__
#define MAYBE_UNUSED __attribute__((unused))
#else
// VC++ has no equivalent
#define MAYBE_UNUSED
#endif

void check_best_overall_error(const color_cell_compressor_params *pParams, color_cell_compressor_results *pResults)
{
const uint32_t n = pParams->m_num_selector_weights;
Expand All @@ -1301,12 +1310,12 @@ void check_best_overall_error(const color_cell_compressor_params *pParams, color
for (uint32_t c = 0; c < 4; c++)
colors[i].m_c[c] = (uint8_t)astc_interpolate_linear(colors[0].m_c[c], colors[n - 1].m_c[c], pParams->m_pSelector_weights[i]);

uint64_t total_err = 0;
MAYBE_UNUSED uint64_t total_err = 0;
for (uint32_t p = 0; p < pParams->m_num_pixels; p++)
{
const color_quad_u8 &orig = pParams->m_pPixels[p];
const color_quad_u8 &packed = colors[pResults->m_pSelectors[p]];

if (pParams->m_has_alpha)
total_err += compute_color_distance_rgba(&orig, &packed, pParams->m_perceptual, pParams->m_weights);
else
Expand Down
3 changes: 1 addition & 2 deletions lib/basisu/encoder/basisu_comp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1303,13 +1303,12 @@ namespace basisu

m_output_basis_file = comp_data;

uint32_t total_orig_pixels = 0, total_texels = 0, total_orig_texels = 0;
uint32_t total_orig_pixels = 0, total_orig_texels = 0;
for (uint32_t i = 0; i < m_slice_descs.size(); i++)
{
const basisu_backend_slice_desc& slice_desc = m_slice_descs[i];

total_orig_pixels += slice_desc.m_orig_width * slice_desc.m_orig_height;
total_texels += slice_desc.m_width * slice_desc.m_height;
}

m_basis_file_size = (uint32_t)comp_data.size();
Expand Down
4 changes: 0 additions & 4 deletions lib/basisu/encoder/basisu_frontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2346,8 +2346,6 @@ namespace basisu
if (!cluster_block_indices.size())
continue;

uint64_t overall_best_err = 0;

uint64_t total_err[4][4][4];
clear_obj(total_err);

Expand Down Expand Up @@ -2398,8 +2396,6 @@ namespace basisu
}

m_optimized_cluster_selectors[cluster_index].set_selector(x, y, best_sel);

overall_best_err += best_err;
} // x
} // y

Expand Down
4 changes: 0 additions & 4 deletions lib/basisu/encoder/basisu_gpu_texture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1533,17 +1533,13 @@ namespace basisu
packed_uint<4> packed_img_size(img_size);
append_vector(ktx_data, (uint8_t *)&packed_img_size, sizeof(packed_img_size));

uint32_t bytes_written = 0;

for (uint32_t array_index = 0; array_index < maximum<uint32_t>(1, header.m_numberOfArrayElements); array_index++)
{
for (uint32_t face_index = 0; face_index < header.m_numberOfFaces; face_index++)
{
const gpu_image& img = gpu_images[cubemap_flag ? (array_index * 6 + face_index) : array_index][level_index];

append_vector(ktx_data, (uint8_t *)img.get_ptr(), img.get_size_in_bytes());

bytes_written += img.get_size_in_bytes();
}

} // array_index
Expand Down
12 changes: 11 additions & 1 deletion lib/basisu/encoder/basisu_uastc_enc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -383,13 +383,17 @@ namespace basisu
}
}

#if UASTC_WRITE_MODE_DESCS
uint32_t total_endpoint_bits = 0;
#endif

for (uint32_t i = 0; i < total_tq_values; i++)
{
const uint32_t num_bits = ep_trits ? 8 : 7;
uastc_write_bits(buf, block_bit_offset, tq_values[i], num_bits, "ETQ");
#if UASTC_WRITE_MODE_DESCS
total_endpoint_bits += num_bits;
#endif
}

if (tq_mul > 1)
Expand All @@ -414,20 +418,24 @@ namespace basisu
num_bits = 5;
}
uastc_write_bits(buf, block_bit_offset, tq_accum, num_bits, "ETQ");
#if UASTC_WRITE_MODE_DESCS
total_endpoint_bits += num_bits;
#endif
}

for (uint32_t i = 0; i < total_values; i++)
{
uastc_write_bits(buf, block_bit_offset, bit_values[i], ep_bits, "EBITS");
#if UASTC_WRITE_MODE_DESCS
total_endpoint_bits += ep_bits;
#endif
}

#if UASTC_WRITE_MODE_DESCS
uint32_t weight_start = block_bit_offset;
uint32_t total_weight_bits = 0;
#endif

uint32_t total_weight_bits = 0;
const uint32_t plane_shift = (total_planes == 2) ? 1 : 0;
for (uint32_t i = 0; i < 16 * total_planes; i++)
{
Expand All @@ -443,7 +451,9 @@ namespace basisu

uastc_write_bits(buf, block_bit_offset, weights[i], numbits, nullptr);

#if UASTC_WRITE_MODE_DESCS
total_weight_bits += numbits;
#endif
}

#if UASTC_WRITE_MODE_DESCS
Expand Down
4 changes: 1 addition & 3 deletions lib/etcdec.cxx
Original file line number Diff line number Diff line change
Expand Up @@ -177,9 +177,7 @@ submitted to the exclusive jurisdiction of the Swedish Courts.
#elif __clang__
#pragma clang diagnostic push
#pragma clang diagnostic ignored "-Wunused-parameter"
#if defined(EMSCRIPTEN)
// Emscripten 3.1.6 lang warns this but not clang 13 in Xcode and Xcode
// warns no such warning group if this is set, hence the extra ifdef.
#if __clang_major__ > 13 || (__clang_major__ == 13 && __clang_minor__ >= 1)
#pragma clang diagnostic ignored "-Wunused-but-set-variable"
#endif
#elif __GNUC__
Expand Down

0 comments on commit fa3d0fd

Please sign in to comment.