halide / Halide

a language for fast, portable data-parallel computation
https://halide-lang.org
Other
5.9k stars 1.07k forks source link

Ambiguous overloaded operator== in C++20 #5329

Closed dbs4261 closed 3 years ago

dbs4261 commented 4 years ago

When compiling a simple halide example, clang-10 presents an error:

In file included from /home/developer/CLionProjects/halide-test/main.cpp:20:
/usr/include/Halide.h:27922:27: error: use of overloaded operator '==' is ambiguous (with operand types 'const Halide::Type' and 'halide_type_t')
            return e.type == type && value == val.u.i64;
                   ~~~~~~ ^  ~~~~
/usr/include/Halide.h:562:31: note: candidate function (with reversed parameter order)
    HALIDE_ALWAYS_INLINE bool operator==(const halide_type_t &other) const {
                              ^
/usr/include/Halide.h:3590:10: note: candidate function
    bool operator==(const Type &other) const {

This seems to be caused by changing set(CMAKE_CXX_STANDARD 17) to set(CMAKE_CXX_STANDARD 20) This can be easily replicated using the first tutorial, and a cmake file as simple as this:

cmake_minimum_required(VERSION 3.17)
project(halide_test)

set(CMAKE_CXX_STANDARD 17)
set(CMAKE_CXX_STANDARD_REQUIRED YES)
set(CMAKE_CXX_EXTENSIONS NO)

find_package(Halide REQUIRED)
add_executable(halide_test lesson_01_basics.cpp)
target_link_libraries(halide_test PRIVATE Halide::Halide)
steven-johnson commented 4 years ago

When you say clang-10, do you mean "I am building Halide using LLVM 10", or "I am building Halide using my local Clang compiler, which happens to be v10"?

steven-johnson commented 4 years ago

So far I have been unable to reproduce this locally. Can you provide more specific info about your specific build environment?

steven-johnson commented 4 years ago

(BTW, I presume this is related to the changes in comparison ops in C++20, e.g. https://brevzin.github.io/c++/2019/07/28/comparisons-cpp20/, and I can see how the ambiguity is occurring, but I haven't figured out how to trigger the warning locally. Are there new warning flags I need to enable, perhaps?)

dbs4261 commented 4 years ago

Im guessing you're right about the comparison operators. Here are the required components of my configuration to reproduce the error as a docker container:

# Build with docker build . -t github-test
# Run with docker run -it github-test /bin/bash to explore the container
# From there you can run `make -C /opt/Test/cmake-build-debug/` to reproduce the error.

FROM ubuntu:bionic

RUN apt-get update && apt-get install -y --no-install-recommends \
    lsb-release wget software-properties-common gpg-agent \
  && wget https://apt.llvm.org/llvm.sh -O /tmp/llvm.sh && chmod +x /tmp/llvm.sh \
  && /tmp/llvm.sh 10 \
  && update-alternatives --install /usr/bin/cc cc /usr/bin/clang-10 999 \
  && update-alternatives --install /usr/bin/c++ c++ /usr/bin/clang++-10 999 \
  && rm -rf /var/lib/apt/lists/*

ARG HALIDE_ARCHIVE_NAME="Halide-10.0.0-x86-64-linux-db901f7f7084025abc3cbb9d17b0f2d3f1745900.tar.gz"
RUN wget https://github.com/halide/Halide/releases/download/v10.0.0/${HALIDE_ARCHIVE_NAME} -O /tmp/$HALIDE_ARCHIVE_NAME \
  && tar -xzf /tmp/$HALIDE_ARCHIVE_NAME -C /tmp/ \
  && rm /tmp/$HALIDE_ARCHIVE_NAME \
  && chmod -R 755 /tmp/Halide-10.0.0-x86-64-linux/ \
  && cp -r /tmp/Halide-10.0.0-x86-64-linux/* /usr/ \
  && rm -rf /tmp/Halide-10.0.0-x86-64-linux/ \
  && chmod -R 755 /usr/share/Halide/ \
  && chmod -R 755 /usr/share/doc/ \
  && chmod -R 755 /usr/lib/cmake/Halide \
  && chmod 755 /usr/lib/libHalide* \
  && chmod 755 /usr/lib/libautoschedule* \
  && chmod 755 /usr/include/Halide* \
  && chmod 755 /usr/bin/weightsdir_to_weightsfile \
  && chmod 755 /usr/bin/retrain_cost_model \
  && chmod 755 /usr/bin/get_host_target \
  && chmod 755 /usr/bin/featurization_to_sample

ARG EXAMPLE_DEPENDENCIES="true"
RUN if [ "$EXAMPLE_DEPENDENCIES" = "true" ] ; then \
    apt-get update && apt-get install -y --no-install-recommends \
      libpng-dev libjpeg-dev python3-dev \
      python3-numpy python3-scipy python3-imageio python3-pybind11 \
      libopenblas-dev libeigen3-dev libatlas-base-dev \
    && rm -rf /var/lib/apt/lists/* ;\
  fi

RUN ldconfig -v

RUN apt-get purge cmake \
  && wget https://github.com/Kitware/CMake/releases/download/v3.17.5/cmake-3.17.5-Linux-x86_64.tar.gz -O /tmp/cmake-3.17.5-Linux-x86_64.tar.gz \
  && tar -xzf /tmp/cmake-3.17.5-Linux-x86_64.tar.gz -C /tmp/ \
  && rm /tmp/cmake-3.17.5-Linux-x86_64.tar.gz \
  && cp -r /tmp/cmake-3.17.5-Linux-x86_64/* /usr/ \
  && rm -rf /tmp/cmake-3.17.5-Linux-x86_64/

RUN apt-get update && apt-get install -y --no-install-recommends git \
  && git clone https://github.com/halide/Halide.git /opt/Halide/ \
  && mkdir -p /opt/Test/ \
  && cp /opt/Halide/tutorial/lesson_01_basics.cpp /opt/Test/lesson_01_basics.cpp \
  && printf "cmake_minimum_required(VERSION 3.17)\nproject(halide_test)\n\nset(CMAKE_CXX_STANDARD 20)\nset(CMAKE_CXX_STANDARD_REQUIRED YES)\nset(CMAKE_CXX_EXTENSIONS NO)\n\nfind_package(Halide REQUIRED)\nadd_executable(halide_test lesson_01_basics.cpp)\ntarget_link_libraries(halide_test PRIVATE Halide::Halide)\n" >> /opt/Test/CMakeLists.txt \
  && mkdir -p /opt/Test/cmake-build-debug/ \
  && cmake -H/opt/Test/ -B/opt/Test/cmake-build-debug/ -DCMAKE_BUILD_TYPE=DEBUG \
  && rm -rf /var/lib/apt/lists/*
steven-johnson commented 4 years ago

Thanks.

(FWIW: we don't even require C++17 yet, and I don't think any of the core contributors are actively building with C++20 at this time, so it may be a bit before we get to this. We'd definitely welcome a patch to fix this!)

soufianekhiat commented 3 years ago

Reproducible with: Halide-12.0.0-x86-64-windows-bc42da9579f08abd6bc4dd5bfbdb4a925662c361 With Visual Studio:

soufianekhiat commented 3 years ago

Update & fix: Build with /std:c++17 but not with /std:c++latest on On 16.9.2. Fix just use explicit cast: return (halide_type_t)e.type == type && value == val.u.i64; Instead of: return e.type == type && value == val.u.i64;

And in all other cases: WildConstUInt, ...