CGAL / cgal

The public CGAL repository, see the README below
https://github.com/CGAL/cgal#readme
Other
5.04k stars 1.39k forks source link

Warnings in master for 5.4 #6108

Closed sloriot closed 1 year ago

sloriot commented 3 years ago

Windows

afabri commented 3 years ago

This one "CGAL/Arr_geometry_traits/Conic_x_monotone_arc_2.h:205:42: warning: bitwise operation between different enumeration types" is in Qt, not in CGAL.

lrineau commented 3 years ago

This one "CGAL/Arr_geometry_traits/Conic_x_monotone_arc_2.h:205:42: warning: bitwise operation between different enumeration types" is in Qt, not in CGAL.

There are indeed similar warnings in Qt, but the CGAL warning is:

/mnt/testsuite/include/CGAL/Arr_geometry_traits/Conic_x_monotone_arc_2.h:205:42: warning: bitwise operation between different enumeration types ('CGAL::_Conic_arc_2<CGAL::Cartesian<CORE::BigRat>, CGAL::Cartesian<CORE::Expr>, CGAL::CORE_algebraic_number_traits>::(anonymous enum at /mnt/testsuite/include/CGAL/Arr_geometry_traits/Conic_arc_2.h:81:3)' and 'CGAL::_Conic_x_monotone_arc_2<CGAL::_Conic_arc_2<CGAL::Cartesian<CORE::BigRat>, CGAL::Cartesian<CORE::Expr>, CGAL::CORE_algebraic_number_traits>>::(anonymous enum at /mnt/testsuite/include/CGAL/Arr_geometry_traits/Conic_x_monotone_arc_2.h:85:3)') is deprecated [-Wdeprecated-anon-enum-enum-conversion]
    this->_info = (Conic_arc_2::IS_VALID | DEGREE_1);
                   ~~~~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~

See https://cgal.geometryfactory.com/CGAL/testsuite/CGAL-5.4-I-84/Arrangement_on_surface_2_Demo/TestReport_lrineau_ArchLinux-clang-CXX20-Release.gz

afabri commented 3 years ago

So let me apply a static_cast<int> before or-ing.

lrineau commented 3 years ago

The target variable this->_info is of type int, anyway.

lrineau commented 3 years ago

@danston I do not understand the deprecation warnings in Barycentric_coordinates_2. Are they false positives?

danston commented 3 years ago

@lrineau Yes. I tried to fix them but nothing helped. See here.

jasjuang commented 3 years ago

Here are a few more warnings on Windows:

1. https://github.com/CGAL/cgal/blob/daf70dbcf6d400cbece30ae173e3e951e48aed0a/Triangulation_2/include/CGAL/Constrained_triangulation_2.h#L1021

warning C4702: unreachable code

Can we simply remove line 1021?

2. https://github.com/CGAL/cgal/blob/1082f223db46829c6bcfc0e0f338593275453a38/Polygon_mesh_processing/include/CGAL/Polyhedral_envelope.h#L758 and https://github.com/CGAL/cgal/blob/1082f223db46829c6bcfc0e0f338593275453a38/Polygon_mesh_processing/include/CGAL/Polyhedral_envelope.h#L906

warning C4701: potentially uninitialized local variable 'ori' used

Can we initialize ori to something at https://github.com/CGAL/cgal/blob/1082f223db46829c6bcfc0e0f338593275453a38/Polygon_mesh_processing/include/CGAL/Polyhedral_envelope.h#L741 and https://github.com/CGAL/cgal/blob/1082f223db46829c6bcfc0e0f338593275453a38/Polygon_mesh_processing/include/CGAL/Polyhedral_envelope.h#L891

3. https://github.com/CGAL/cgal/blob/master/Intersections_3/include/CGAL/Intersections_3/internal/Bbox_3_Triangle_3_do_intersect.h#L258

warning C4702: unreachable code

Can we simply remove line 258?

jasjuang commented 3 years ago

Another warning in Visual Studio:

https://github.com/CGAL/cgal/blob/master/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h#L935

warning C4701: potentially uninitialized local variable 'deviation_post' used

Not initializing deviation_post in line 924 looks like it could cause undesired behavior.

https://github.com/CGAL/cgal/blob/master/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h#L924

lrineau commented 3 years ago

Another warning in Visual Studio:

https://github.com/CGAL/cgal/blob/master/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h#L935

warning C4701: potentially uninitialized local variable 'deviation_post' used

Not initializing deviation_post in line 924 looks like it could cause undesired behavior.

https://github.com/CGAL/cgal/blob/master/Polygon_mesh_processing/include/CGAL/Polygon_mesh_processing/internal/Isotropic_remeshing/remesh_impl.h#L924

Only the usage of an uninitialized variable is an undefined behavior. Here the initialization and usage of deviation_post is coupled to the condition !badly_shaped, and thus the code is correct. Can you please tell us:

jasjuang commented 3 years ago

@lrineau here are the steps to reproduce it on Visual Studio 2019, Windows 10

vcpkg install cgal

CMakeLists.txt

set(CMAKE_TOOLCHAIN_FILE $ENV{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake)

cmake_minimum_required(VERSION 3.20)

project(example)

set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /W4 /WX")

find_package(CGAL REQUIRED)

add_executable(${PROJECT_NAME} main.cpp)

target_link_libraries(${PROJECT_NAME} PUBLIC CGAL::CGAL)

main.cpp

__pragma(warning(push,0))
#include "CGAL/Polygon_mesh_processing/remesh.h"
#include "CGAL/Surface_mesh.h"
__pragma(warning(pop))

int main() { 
    CGAL::Surface_mesh<CGAL::Exact_predicates_inexact_constructions_kernel::Point_3> mesh;

    double tar_len;

    CGAL::Polygon_mesh_processing::isotropic_remeshing(mesh.faces(), tar_len, mesh);

    return 0; 
}

This results in compilation error:

1>------ Build started: Project: example, Configuration: Debug x64 ------
1>main.cpp
1>C:\vcpkg\installed\x64-windows\include\CGAL\Polygon_mesh_processing\internal\Isotropic_remeshing\remesh_impl.h(942): error C2220: the following warning is treated as an error
1>C:\vcpkg\installed\x64-windows\include\CGAL\Polygon_mesh_processing\internal\Isotropic_remeshing\remesh_impl.h(942): warning C4701: potentially uninitialized local variable 'deviation_post' used
1>Done building project "example.vcxproj" -- FAILED.

This warning/error will go away if I modify line 924 to

int deviation_post;

to

int deviation_post = std::numeric_limits<int>::max();
lrineau commented 3 years ago

Hi @jasjuang, now I understand. In the CGAL daily testsuite, we have several columns corresponding to Windows platforms, and on all of them we only use /W3, and /W4. That is why CGAL developers are not aware of warnings of level 4, like warning C4701: potentially uninitialized local variable. I will discuss with my colleagues, to decide if we want to support /W4 on Windows.

sloriot commented 1 year ago

This issue is pretty old now. A new one should be created with the current warnings.