llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
28.64k stars 11.84k forks source link

Using `auto` keyword bypasses `if constexpr` validation rules #61300

Open paulsemel opened 1 year ago

paulsemel commented 1 year ago

The following code compiles correctly (https://godbolt.org/z/98dr4bsT8):

#include <type_traits>
template <typename T>
class raw_ptr {
 public:
  T* get() const { return nullptr; }
  void ReportIfDangling() const {}
  operator T*() const { return nullptr; }
};

template <typename T>
struct IsRawPtr : std::false_type {};

template <typename T>
struct IsRawPtr<raw_ptr<T>> : std::true_type {};

template <typename T>
inline constexpr bool IsRawPtrV = IsRawPtr<T>::value;

template <typename T>
class UnretainedWrapper {
 public:
  T* get() const {
    // `ptr_` is either a `raw_ptr` or a regular C++ pointer.
    if constexpr (IsRawPtrV<StorageType>) {
        const auto& type = ptr_;
        type.ReportIfDangling();
    }
    return ptr_;
  }

  using StorageType = T*;
//   using StorageType = raw_ptr<T>;
  StorageType ptr_;
};

int main() {
  UnretainedWrapper<int> w;
  w.get();
  return 0;
}

Although, this doesn't respect https://en.cppreference.com/w/cpp/language/if:

Outside a template, a discarded statement is fully checked

Changing the get method to:

  T* get() const {
    // `ptr_` is either a `raw_ptr` or a regular C++ pointer.
    if constexpr (IsRawPtrV<StorageType>) {
        ptr_.ReportIfDangling();
    }
    return ptr_;
  }

will have the expected behaviour.

llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-frontend

danakj commented 1 year ago

Note that GCC and MSVC accept the following, which uses operator. on T* in the never-used constexpr branch.

template <typename T>
class UnretainedWrapper {
 public:
  T* get() const {
    // `ptr_` is either a `raw_ptr` or a regular C++ pointer.
    if constexpr (IsRawPtrV<StorageType>) {
        ptr.ReportIfDangling();
    }
    return ptr_;
  }

  using StorageType = T*;
//   using StorageType = raw_ptr<T>;
  StorageType ptr_;
};

And if not using a type alias, clang accepts the code as well. The StorageType as an alias causes clang to reject the code unlike gcc and MSVC.

This compiles when StorageType is T* in clang (https://godbolt.org/z/nT13bTrfe):

template <typename T, typename StorageType>
class UnretainedWrapper {
 public:
  T* get() const {
    // `ptr_` is either a `raw_ptr` or a regular C++ pointer.
    if constexpr (IsRawPtrV<StorageType>) {
        ptr.ReportIfDangling();
    }
    return ptr_;
  }

  StorageType ptr_;
};
danakj commented 1 year ago

Here's a more minimal example.

This compiles on clang/gcc/msvc: https://godbolt.org/z/KbGrK1P4f

#include <type_traits>

template <typename T>
class raw_ptr {
   public:
    T* get() const { return nullptr; }
};

template <typename T>
struct IsRawPtr : std::false_type {};
template <typename T>
struct IsRawPtr<raw_ptr<T>> : std::true_type {};
template <typename T>
inline constexpr bool IsRawPtrV = IsRawPtr<T>::value;

template <typename T, typename StorageType>
class S {
   public:
    T* get() const {
        if constexpr (IsRawPtrV<StorageTypeAlias>) {
            // If `ptr_` is `T*` this branch does't happen.
            return ptr_.get();
        } else {
            return ptr_;
        }
    }

    using StorageTypeAlias = StorageType;
    StorageTypeAlias ptr_;
};

int main() {
    S<int, int*>().get();
    S<int, raw_ptr<int>>().get();
    return 0;
}

Only Clang rejects this code: https://godbolt.org/z/7xY3sGeM9

#include <type_traits>

template <typename T>
class raw_ptr {
   public:
    T* get() const { return nullptr; }
};

template <typename T>
struct IsRawPtr : std::false_type {};
template <typename T>
struct IsRawPtr<raw_ptr<T>> : std::true_type {};
template <typename T>
inline constexpr bool IsRawPtrV = IsRawPtr<T>::value;

template <typename T, typename /*StorageType*/>
class S {
   public:
    T* get() const {
        if constexpr (IsRawPtrV<StorageTypeAlias>) {
            // If `ptr_` is `T*` this branch does't happen.
            return ptr_.get();
        } else {
            return ptr_;
        }
    }

    using StorageTypeAlias = T*;
    StorageTypeAlias ptr_;
};

int main() {
    S<int, int*>().get();
    S<int, raw_ptr<int>>().get();
    return 0;
}

The type of ptr_ is no longer a template parameter of the class. Is gcc and MSVC accepting the code incorrectly?