[PATCH] D55552: [Sema] Better static assert diagnostics for expressions involving temporaries.

Clement Courbet via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 12 09:14:11 PST 2018


courbet planned changes to this revision.
courbet marked an inline comment as done.
courbet added inline comments.


================
Comment at: test/SemaCXX/static-assert.cpp:136
+static_assert(std::is_const<decltype(ExampleTypes(3))>::value, "message");
+// expected-error at -1{{static_assert failed due to requirement 'std::is_const<ExampleTypes>::value' "message"}}
 
----------------
Quuxplusone wrote:
> Conspicuously missing any test for lines 3081–3106 above. IIUC, those lines would trigger on things like
> 
> ```
> template<class T> struct X { int i=0, j=0; constexpr operator bool() const { return false; } };
> template<class T> void test() {
>     static_assert(X<T>{});
>     static_assert(X<T>{1,2});
>     static_assert(T{0});
>     static_assert(T(0));
> }
> template void test<int>();
> ```
> 
> But I guess I don't see why extra code is needed to handle those; shouldn't the pretty-printer handle them already? What do the current diagnostics look like for my example?
Note: Your examples are `CXXFunctionalCastExpr`, and `CXXUnresolvedConstructExpr` respectively, not `CXXTemporaryObjectExpr`, so they are not handled by this change.

They are correctly printed before this change with `T==int`.

The issue comes when adding an extra layer of indirection:

```
struct ExampleTypes {
  explicit ExampleTypes(int);
  using T = int;
  using U = float;
};

template<class T> struct X {
  int i=0;
  int j=0;
  constexpr operator bool() const { return false; }
};

template<class T> void foo6() {
    static_assert(X<typename T::T>());
    static_assert(X<typename T::T>{});
    static_assert(X<typename T::T>{1,2});
    static_assert(X<typename T::T>({1,2}));
    static_assert(typename T::T{0});
    static_assert(typename T::T(0));
}
template void foo6<ExampleTypes>();
```

The errors before the change are:
```
File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 71: static_assert failed due to requirement 'X<typename ExampleTypes::T>()'
  File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 72: static_assert failed due to requirement 'X<typename ExampleTypes::T>{}'
  File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 73: static_assert failed due to requirement 'X<typename ExampleTypes::T>{1, 2}'
  File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 74: static_assert failed due to requirement 'X<typename ExampleTypes::T>({1, 2})'
  File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 75: static_assert failed due to requirement 'typename ExampleTypes::T{0}'
  File /usr/local/google/home/courbet/llvm/llvm/tools/clang/test/SemaCXX/static-assert-cxx17.cpp Line 76: static_assert failed due to requirement 'typename ExampleTypes::T(0)'
```

I must admit that I did not think of handling these, and now I think it would actually be better to tap into the pretty printer to avoid code duplication. 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55552/new/

https://reviews.llvm.org/D55552





More information about the llvm-commits mailing list