[Clang] Implement CWG2813: Class member access with prvalues #120223
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
Create Sema::DiagnoseDiscardedNodiscard Remove warn_discarded_class_member_access and call Sema::DiagnoseDiscardedNodiscard instead Remove MakeGLValue from the MSPropertyRefExpr path Fix [[nodiscard]] test in test/CXX Add test for [[nodiscard]] and explicit object member functions in test/SemaCXX
…-explicit-object-member-function-call
cor3ntin
requested review from
Sirraide and
erichkeane
and removed request for
Endilll
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
Sirraide
reviewed
Dec 17, 2024
clang/lib/Sema/SemaStmt.cpp
Outdated
Comment on lines
404
to
407
| // Don't diagnose discarded left of dot in static class member access | ||
| // because its type is "used" to determine the class to access | ||
| //if (DiagID == diag::warn_discarded_class_member_access) | ||
| // return; |
There was a problem hiding this comment.
I think this can be deleted at this point?
cor3ntin
added
the
clang:frontend
erichkeane
approved these changes
Dec 17, 2024
clang/include/clang/Sema/Sema.h
Outdated
| @@ -10659,6 +10659,11 @@ class Sema final : public SemaBase { | |||
| SourceLocation EndLoc); | |||
| void ActOnForEachDeclStmt(DeclGroupPtrTy Decl); | |||
|
|
|||
| /// DiagnoseDiscardedExprMarkedNodiscard - Given an expression that is | |||
| /// semantically a discarded-value expression, diagnose if any [[nodiscard]] | |||
| /// value has been discarded | |||
There was a problem hiding this comment.
Suggested change
| /// value has been discarded | |
| /// value has been discarded. |
clang/lib/Sema/SemaExprMember.cpp
Outdated
| // an enumerator, the first expression is a discarded-value expression; if | ||
| // the id-expression names a non-static data member, the first expression | ||
| // shall be a glvalue. | ||
| auto MakeDiscardedValue = [&] { |
There was a problem hiding this comment.
I would prefer some more descriptive names here? Perhaps ConvertBaseExprToDiscardedValue and ConvertBaseExprToGLValue ?
searlmc1
pushed a commit
to ROCm/llvm-project
that referenced
this pull request
Dec 18, 2024
…0223) This is a rebase of llvm#95112 with my own feedback apply as @MitalAshok has been inactive for a while. It's fairly important this makes clang 20 as it is a blocker for llvm#107451 Change-Id: I2261810f7c8d7dd8b3e3412c0814a528d7c7b91c --- [CWG2813](https://cplusplus.github.io/CWG/issues/2813.html) prvalue.member_fn(expression-list) now will not materialize a temporary for prvalue if member_fn is an explicit object member function, and prvalue will bind directly to the object parameter. The E1 in E1.static_member is now a discarded-value expression, so if E1 was a call to a [[nodiscard]] function, there will now be a warning. This also affects C++98 with [[gnu::warn_unused_result]] functions. This should not affect C where TemporaryMaterializationConversion is a no-op. Closes llvm#100314 Fixes llvm#100341 --------- Co-authored-by: Mital Ashok <[email protected]> (cherry picked from commit db93ef1) Change-Id: Ic277d16bc8611b9d383cb890da3eda0ef1646555
bherrera
pushed a commit
to misttech/mist-os
that referenced
this pull request
Jan 6, 2025
llvm/llvm-project#120223 became more strict about discarded value expressions. The current std::forward expressions triggered a no-discard warning/error. It seems like we can remove the `std::forward` on the lvalue reference in these visitor functions anyway to resolve this error. Fixed: 385748190 Change-Id: Iaf51848f82f0979f72a26736d33b50ad60738de4 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1179435 Commit-Queue: Auto-Submit <[email protected]> Fuchsia-Auto-Submit: Caslyn Tonelli <[email protected]> Reviewed-by: Casey Dahlin <[email protected]>
bherrera
pushed a commit
to misttech/mist-os
that referenced
this pull request
Jan 6, 2025
llvm/llvm-project#120223 became more strict about discarded value expressions. This CL moves the array reference into a local variable to operate on. Bug: 385748190 Change-Id: Iafe9ff737500c7bab8f55d3e216c5606048ab4c2 Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1179388 Reviewed-by: Justin Mattson <[email protected]> Fuchsia-Auto-Submit: Caslyn Tonelli <[email protected]> Commit-Queue: Auto-Submit <[email protected]>
bherrera
pushed a commit
to misttech/mist-os
that referenced
this pull request
Jan 6, 2025
llvm/llvm-project#120223 became more strict about discarded value expressions. This CL moves the array reference into a local variable to operate on. Bug: 385748190 Change-Id: I468bc0e122778a867ea452535be5d4562802264e Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1179632 Reviewed-by: Karl Ward <[email protected]> Commit-Queue: Auto-Submit <[email protected]> Fuchsia-Auto-Submit: Caslyn Tonelli <[email protected]>
bherrera
pushed a commit
to misttech/integration
that referenced
this pull request
Jan 6, 2025
llvm/llvm-project#120223 became more strict about discarded value expressions. The current std::forward expressions triggered a no-discard warning/error. It seems like we can remove the `std::forward` on the lvalue reference in these visitor functions anyway to resolve this error. Original-Fixed: 385748190 Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1179435 Original-Revision: f5151dcdb438a72b806da2dd8efdf1dcefe45583 GitOrigin-RevId: 1da2bddea99f38cf500975599e0a17cf1e649236 Change-Id: I053bca01ca47a110a9e3ccd29d3c8ce19f7588ef
bherrera
pushed a commit
to misttech/integration
that referenced
this pull request
Jan 6, 2025
llvm/llvm-project#120223 became more strict about discarded value expressions. This CL moves the array reference into a local variable to operate on. Original-Bug: 385748190 Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1179388 Original-Revision: 24958deb6cd71533593a4be6cb4b339b0f69be63 GitOrigin-RevId: a1e1527c35b8fc8e4368ff974ea5ac3f3108fc80 Change-Id: I84e72196e08e5372cda062963625d51d5b67e629
bherrera
pushed a commit
to misttech/integration
that referenced
this pull request
Jan 6, 2025
llvm/llvm-project#120223 became more strict about discarded value expressions. This CL moves the array reference into a local variable to operate on. Original-Bug: 385748190 Original-Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1179632 Original-Revision: bac0bfd54ea3efbc387ccf36a5a8e2e99debce0c GitOrigin-RevId: d4eb0af1986902cf6b06628fb3f5965e68d9b45b Change-Id: I97d076d5263521cf69e59f9e8a479dbeba2fa148
|
It looks like this is linked to: #131410 and it is not clear to me the diagnostic in these cases makes a lot of sense or at least the motivation does not feel strong to me. This new approach feels evolutionary to me and if this was the intent then I am not sure we should implement it. |
cor3ntin
added a commit
to cor3ntin/llvm-project
that referenced
this pull request
Mar 15, 2025
…mber access For an expression `nodiscard_function().static_member(), the nodiscard warnings added by llvm#120223, are not useful or actionable, and are disruptive to some library implementations; we just remove them. Fixes llvm#131410
cor3ntin
added a commit
to cor3ntin/llvm-project
that referenced
this pull request
Mar 18, 2025
…mber access (llvm#131450) For an expression `nodiscard_function().static_member(), the nodiscard warnings added by llvm#120223, are not useful or actionable, and are disruptive to some library implementations; we just remove them. Fixes llvm#131410 (cherry picked from commit 9a1e390)
swift-ci
pushed a commit
to swiftlang/llvm-project
that referenced
this pull request
Mar 18, 2025
…mber access (llvm#131450) For an expression `nodiscard_function().static_member(), the nodiscard warnings added by llvm#120223, are not useful or actionable, and are disruptive to some library implementations; we just remove them. Fixes llvm#131410 (cherry picked from commit 9a1e390)
This was referenced Jun 2, 2025
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.
This is a rebase of #95112 with my own feedback apply as @MitalAshok has been inactive for a while.
It's fairly important this makes clang 20 as it is a blocker for #107451
CWG2813
prvalue.member_fn(expression-list) now will not materialize a temporary for prvalue if member_fn is an explicit object member function, and prvalue will bind directly to the object parameter.
The E1 in E1.static_member is now a discarded-value expression, so if E1 was a call to a [[nodiscard]] function, there will now be a warning. This also affects C++98 with [[gnu::warn_unused_result]] functions.
This should not affect C where TemporaryMaterializationConversion is a no-op.
Closes #100314
Fixes #100341