[PATCH] D111400: [Clang][C++2b] P2242R3: Non-literal variables [...] in constexpr

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

> I think labels can be left as is.

Yes, the `static`, `thread_local`, label, and `goto` cases would all categorically contribute to a IFNDR program in the older modes (and we currently reliably diagnose them in the definition context).



================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:1904-1906
+        if (!SemaRef.LangOpts.CPlusPlus2b &&
+            CheckLiteralType(SemaRef, Kind, VD->getLocation(), VD->getType(),
                              diag::err_constexpr_local_var_non_literal_type,
----------------
cor3ntin wrote:
> hubert.reinterpretcast wrote:
> > This seems to unconditionally error in pre-C++2b modes. For consistency, this should be a `-Wc++2b-extensions` warning.
> We agreed not have this an extension in earlier modes because a valid c++20 program can sfinea on that. Is it not a direction you agree with anymore>
I think, now that we have the lambda cases all working, we can be confident that `return false;` in older modes is what this code is responsible for in the SFINAE case.

For handling SFINAE-related cases, there may (eventually) be some restructuring to allow focus on only the template-dependent cases, but C++23 might end up needing to do the checking even for non-dependent cases during constant expression evaluation (because the template definition becomes well-formed due to pending removal of IFNDR).

I've also found indications that Clang generally fails to perform strict constant expression evaluation in relation to instantiations that fail to satisfy the requirements for a constexpr function:
```
template <typename T>
struct A {
  constexpr int f() {
    T t;  // cannot be trivial default initialization until P1331
          // (not DR out of Cologne 2019)
    t = T();
    return 42 + t;
  }
};

template <const int &N>
short g(int, char (*)[N] = 0);

template <const int &N>
long g(void *);

struct Z {
  constexpr Z() {}
  constexpr operator int() { return 0; }
};
const int x = A<int>().f();

extern decltype(g<x>(0)) q;
short q;
```



================
Comment at: clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp:1-3
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++11 -Werror=c++14-extensions -Werror=c++20-extensions -Werror=c++2b-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++14 -DCXX14 -Werror=c++20-extensions -Werror=c++2b-extensions %s
+// RUN: %clang_cc1 -verify -fcxx-exceptions -triple=x86_64-linux-gnu -std=c++20 -DCXX14 -DCXX20 -Werror=c++2b-extensions %s
----------------
cor3ntin wrote:
> hubert.reinterpretcast wrote:
> > The use `-Werror` hides potential issues where the error is categorically produced (instead of under the warning group).
> > 
> > Considering that there is a relevant design consistency issue of exactly that sort right now that this test could have highlighted (but didn't), a change to stop using `-Werror` may be prudent.
> This seems inconsistent with how that file is currently structured though
I meant to change the entire file to check for warnings instead of errors. I guess that really should be a separate patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111400