[clang][ExprConst] Allow non-literal types in C++23 #100062
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Timm Baeder (tbaederr) ChangesInstead of diagnosing non-literal types in C++23, allow them and later diagnose them differently, e.g. because they have a non-constexpr constructor, destructor, etc. For this test: The current diagnostics with c++20/c++23 are: With the As mentioned in #60311, this is confusing. The output with c++20 suggests that using c++23 will make the problem go away, but it's diagnosed the same when running the function. With this commit, the output instead diagnoses why the non-literal type can't be used: Fixes #60311 Full diff: https://github.com/llvm/llvm-project/pull/100062.diff 5 Files Affected:
|
Sirraide
approved these changes
Jul 23, 2024
There was a problem hiding this comment.
LGTM
Considering that the literal type requirement was quite literally just straight-up removed in C++23, returning early here is basically just doing what the standard did, so this change makes sense to me.
Also, one option would be to move this check up even further to be the first thing we check for in this function (don’t know how expensive the isLiteralType check is off the top of my head). I’m not sure it really matters though, so either way seems fine to me.
cor3ntin
approved these changes
Jul 23, 2024
There was a problem hiding this comment.
This seems reasonable. Slightly confusing because we so have several CheckLiteralType functions doing different things.
FYI as far as i can tell, WG21 is itching to get rid of the notion of literal types entirely so this further goes into that direction .
I find the way we print destructor/constructor names in some of these diagnostics rip for improvement. ie function '~NonLiteral' is not amazing. But it's a pre existing issue
We will need a release note though
Instead of diagnosing non-literal types in C++23, allow them and later diagnose them differently, e.g. because they have a non-constexpr constructor, destructor, etc.
yuxuanchen1997
pushed a commit
that referenced
this pull request
Jul 25, 2024
Summary:
Instead of diagnosing non-literal types in C++23, allow them and later
diagnose them differently, e.g. because they have a non-constexpr
constructor, destructor, etc.
For this test:
```c++
struct NonLiteral {
NonLiteral() {}
};
constexpr int foo() {
NonLiteral L;
return 1;
}
// static_assert(foo() == 1);
```
The current diagnostics with c++20/c++23 are:
```console
~/code/llvm-project/build » clang -c array.cpp -std=c++20
array.cpp:91:14: error: variable of non-literal type 'NonLiteral' cannot be defined in a constexpr function before C++23
91 | NonLiteral L;
| ^
array.cpp:87:8: note: 'NonLiteral' is not literal because it is not an aggregate and has no constexpr constructors other than copy or move constructors
87 | struct NonLiteral {
| ^
1 error generated.
------------------------------------------------------------
~/code/llvm-project/build » clang -c array.cpp -std=c++23
(no output)
```
With the `static_assert` enabled, compiling with `-std=c++23` prints:
```console
array.cpp:95:15: error: static assertion expression is not an integral constant expression
95 | static_assert(foo() == 1);
| ^~~~~~~~~~
array.cpp:91:14: note: non-literal type 'NonLiteral' cannot be used in a constant expression
91 | NonLiteral L;
| ^
array.cpp:95:15: note: in call to 'foo()'
95 | static_assert(foo() == 1);
| ^~~~~
1 error generated.
```
As mentioned in #60311, this is confusing. The output with c++20
suggests that using c++23 will make the problem go away, but it's
diagnosed the same when running the function.
With this commit, the output instead diagnoses _why_ the non-literal
type can't be used:
```console
array.cpp:95:15: error: static assertion expression is not an integral constant expression
95 | static_assert(foo() == 1);
| ^~~~~~~~~~
array.cpp:91:14: note: non-constexpr constructor 'NonLiteral' cannot be used in a constant expression
91 | NonLiteral L;
| ^
array.cpp:95:15: note: in call to 'foo()'
95 | static_assert(foo() == 1);
| ^~~~~
array.cpp:88:3: note: declared here
88 | NonLiteral() {}
| ^
1 error generated.
```
Fixes #60311
Test Plan:
Reviewers:
Subscribers:
Tasks:
Tags:
Differential Revision: https://phabricator.intern.facebook.com/D60250647
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Instead of diagnosing non-literal types in C++23, allow them and later diagnose them differently, e.g. because they have a non-constexpr constructor, destructor, etc.
For this test:
The current diagnostics with c++20/c++23 are:
With the
static_assertenabled, compiling with-std=c++23prints:As mentioned in #60311, this is confusing. The output with c++20 suggests that using c++23 will make the problem go away, but it's diagnosed the same when running the function.
With this commit, the output instead diagnoses why the non-literal type can't be used:
Fixes #60311