[Issue] Compiler warnings with -Wextra enabled

[zacharyvincze]

Description

amdclang++ throws warnings when -Wextra is added as a compiler option. The breakdown of the warnings is as follows:

560 [-Wignored-qualifiers]
374 [-Wsign-compare]
 96 [-Wunused-parameter]
 64 [-Wdeprecated-copy-with-user-provided-copy]
 30 [-Wpass-failed=transform-warning]

Suggested plan of action

• Create PRs that address each warning type by category, adding their respective flag into the CMake configuration to confirm that no warnings are thrown.
• Once all sub issues have been resolved, use -Wextra as a compile option and -Werror to ensure compilation cannot succeed if there are any warnings thrown. This will make it easier to catch and address warnings moving forward.

[simonCatBot]

Comprehensive Fix for -Wextra Compiler Warnings

This is the parent issue tracking all compiler warnings. Here's a comprehensive plan to address all warning categories:

Warning Breakdown Summary

Warning Type	Count	Priority	Complexity
-Wignored-qualifiers	560	Low	Easy
-Wsign-compare	374	High	Medium
-Wunused-parameter	96	Medium	Easy
-Wdeprecated-copy	64	Medium	Medium
-Wpass-failed	30	High	High

Category 1: -Wignored-qualifiers (560 warnings)

Problem: Applying qualifiers like const to return types of value types has no effect.

// Warning-triggering code
const int getValue();  // const on return type is ignored
const Image process(); // same

// Fixed
int getValue();
Image process();

Fix Strategy:

# Find and remove const from return types
sed -i 's/const \([a-zA-Z_][a-zA-Z0-9_]*\) \([a-zA-Z_][a-zA-Z0-9_]*\)();/\1 \2();/g'

Category 2: -Wsign-compare (374 warnings)

See Issue #129 for detailed fix. Quick summary:

// Before
for (int i = 0; i < vec.size(); i++)  // int vs size_t

// After  
for (size_t i = 0; i < vec.size(); i++)  // size_t vs size_t
// or
for (auto i = 0u; i < vec.size(); i++)  // unsigned vs size_t

Category 3: -Wunused-parameter (96 warnings)

See Issue #130 for detailed fix. Quick summary:

// Before
void func(int x, int y) { return x; }  // y unused

// After
void func(int x, int /*y*/) { return x; }
// or
void func(int x, [[maybe_unused]] int y) { return x; }

Category 4: -Wdeprecated-copy (64 warnings)

See Issue #131 for detailed fix. Quick summary:

// Before
class MyClass {
    MyClass(const MyClass& other);  // only copy ctor
};

// After
class MyClass {
    MyClass(const MyClass& other) = default;
    MyClass& operator=(const MyClass& other) = default;
    MyClass(MyClass&& other) noexcept = default;
    MyClass& operator=(MyClass&& other) noexcept = default;
};

Category 5: -Wpass-failed=transform-warning (30 warnings)

Problem: This indicates the compiler failed to apply an optimization transformation, often due to code complexity or dependencies.

// May fail auto-vectorization
void process(float* data, int n) {
    for (int i = 0; i < n; i++) {
        data[i] = data[i] * 2.0f;  // Dependency on data[i]
    }
}

// Better: explicit vectorization hints
void process(float* __restrict__ data, int n) {
    #pragma omp simd
    for (int i = 0; i < n; i++) {
        data[i] = data[i] * 2.0f;
    }
}

Comprehensive CMake Configuration

# CMakeLists.txt - Warning configuration

# Option to enable strict warnings
option(ENABLE_STRICT_WARNINGS "Enable -Wextra and treat warnings as errors" OFF)

if(ENABLE_STRICT_WARNINGS)
    if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" OR 
       CMAKE_CXX_COMPILER_ID STREQUAL "Clang" OR
       CMAKE_CXX_COMPILER_ID STREQUAL "AMDClang")
        
        # Enable all extra warnings
        target_compile_options(roccv PRIVATE
            -Wall
            -Wextra
            -Wpedantic
        )
        
        # Specific warning categories (for incremental fixing)
        target_compile_options(roccv PRIVATE
            -Wignored-qualifiers
            -Wsign-compare
            -Wunused-parameter
            -Wdeprecated-copy-with-user-provided-copy
            -Wpass-failed=transform-warning
        )
        
        # Uncomment after fixing all warnings:
        # target_compile_options(roccv PRIVATE -Werror)
    endif()
endif()

# Create separate target for warning-clean code
target_compile_options(roccv_warning_clean PRIVATE
    -Wall -Wextra -Werror
)

Phased Fix Strategy

Phase 1: Low-Hanging Fruit (Week 1)

• Fix -Wignored-qualifiers (easy, high volume)
• Fix -Wunused-parameter (easy, medium volume)

Phase 2: Type Safety (Week 2-3)

• Fix -Wsign-compare (requires careful review)
• Add static assertions for type safety

Phase 3: Code Modernization (Week 4)

• Fix -Wdeprecated-copy (modernize to C++11/14/17)
• Implement Rule of Five where needed

Phase 4: Optimization (Week 5)

• Fix -Wpass-failed warnings
• Optimize hot paths for vectorization

Phase 5: Enforcement (Week 6)

• Enable -Werror in CI
• Add pre-commit hooks

CI/CD Integration

# .github/workflows/build.yml
name: Build with Strict Warnings

jobs:
  build:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v3
      
      - name: Configure with warnings
        run: |
          cmake -B build \
            -DENABLE_STRICT_WARNINGS=ON \
            -DCMAKE_CXX_FLAGS="-Werror"
      
      - name: Build
        run: cmake --build build --parallel
        
      - name: Upload warnings log
        if: failure()
        uses: actions/upload-artifact@v3
        with:
          name: warnings-log
          path: build/compile_commands.json

Recommended Development Workflow

#!/bin/bash
# fix-warnings.sh - Development helper script

WARNING=$1

case $WARNING in
    ignored-qualifiers)
        echo "Fixing ignored-qualifiers..."
        find src -name "*.h" -o -name "*.cpp" | xargs sed -i \
            's/const \([a-zA-Z_][a-zA-Z0-9_]*\) \([a-zA-Z_][a-zA-Z0-9_]*\)();/\1 \2();/g'
        ;;
    unused-parameter)
        echo "Fixing unused-parameter..."
        # Use clang-tidy for automated fixes
        clang-tidy -p build -checks='-*,misc-unused-parameters' -fix \
            src/**/*.cpp
        ;;
    sign-compare)
        echo "Manual review required for sign-compare"
        echo "Run: grep -rn 'warning:.*sign-compare' build.log"
        ;;
    *)
        echo "Unknown warning type"
        ;;
esac

Verification Commands

# Build with warnings enabled
cmake -B build -DENABLE_STRICT_WARNINGS=ON
cmake --build build 2>&1 | tee build.log

# Count warnings by type
grep -o '\[-W[^]]*\]' build.log | sort | uniq -c | sort -rn

# Check for new warnings (compare with baseline)
diff -u baseline-warnings.txt current-warnings.txt

This plan addresses all 1124 warnings systematically while maintaining code quality and performance.

[kiritigowda]

@simonCatBot - PR for Phase 1: Low-Hanging Fruit (Week 1)