Skip to content

[Clang] Do not emit nodiscard warnings for the base expr of static member access #131450

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 15, 2025

Conversation

cor3ntin
Copy link
Contributor

@cor3ntin cor3ntin commented Mar 15, 2025

For an expression `nodiscard_function().static_member(), the nodiscard warnings added by #120223, are not useful or actionable, and are disruptive to some library implementations; we just remove them.

Fixes #131410

…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 cor3ntin marked this pull request as ready for review March 15, 2025 10:16
@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Mar 15, 2025
@cor3ntin
Copy link
Contributor Author

No changelog; it's a good candidate for backport

@llvmbot
Copy link
Member

llvmbot commented Mar 15, 2025

@llvm/pr-subscribers-clang

Author: cor3ntin (cor3ntin)

Changes

…mber access

For an expression `nodiscard_function().static_member(), the nodiscard warnings added by #120223, are not useful or actionable, and are disruptive to some library implementations; we just remove them.

Fixes #131410


Full diff: https://github.com/llvm/llvm-project/pull/131450.diff

5 Files Affected:

  • (modified) clang/include/clang/Sema/Sema.h (-5)
  • (modified) clang/lib/Sema/SemaExprMember.cpp (-1)
  • (modified) clang/lib/Sema/SemaStmt.cpp (-4)
  • (modified) clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp (+6-4)
  • (modified) clang/test/SemaCXX/ms-property.cpp (+1-1)
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index c034de0e633da..9bfc15766c21c 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -10755,11 +10755,6 @@ 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.
-  void DiagnoseDiscardedExprMarkedNodiscard(const Expr *E);
-
   /// DiagnoseUnusedExprResult - If the statement passed in is an expression
   /// whose result is unused, warn.
   void DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID);
diff --git a/clang/lib/Sema/SemaExprMember.cpp b/clang/lib/Sema/SemaExprMember.cpp
index 1d9efbeb5ccb5..b0872122ed740 100644
--- a/clang/lib/Sema/SemaExprMember.cpp
+++ b/clang/lib/Sema/SemaExprMember.cpp
@@ -1136,7 +1136,6 @@ Sema::BuildMemberReferenceExpr(Expr *BaseExpr, QualType BaseExprType,
     if (Converted.isInvalid())
       return true;
     BaseExpr = Converted.get();
-    DiagnoseDiscardedExprMarkedNodiscard(BaseExpr);
     return false;
   };
   auto ConvertBaseExprToGLValue = [&] {
diff --git a/clang/lib/Sema/SemaStmt.cpp b/clang/lib/Sema/SemaStmt.cpp
index 944e3300c83fc..112eaf758cf36 100644
--- a/clang/lib/Sema/SemaStmt.cpp
+++ b/clang/lib/Sema/SemaStmt.cpp
@@ -413,10 +413,6 @@ void DiagnoseUnused(Sema &S, const Expr *E, std::optional<unsigned> DiagID) {
 }
 } // namespace
 
-void Sema::DiagnoseDiscardedExprMarkedNodiscard(const Expr *E) {
-  DiagnoseUnused(*this, E, std::nullopt);
-}
-
 void Sema::DiagnoseUnusedExprResult(const Stmt *S, unsigned DiagID) {
   if (const LabelStmt *Label = dyn_cast_if_present<LabelStmt>(S))
     S = Label->getSubStmt();
diff --git a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
index 18f4bd5e9c0fa..0012ab976baa5 100644
--- a/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
+++ b/clang/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp
@@ -164,19 +164,21 @@ struct X {
 
 [[nodiscard]] X get_X();
 // cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
+[[nodiscard]] X* get_Ptr();
+// cxx11-warning@-1 {{use of the 'nodiscard' attribute is a C++17 extension}}
 void f() {
+  get_X(); // expected-warning{{ignoring return value of function declared with 'nodiscard' attribute}}
+  (void) get_X();
   (void) get_X().variant_member;
   (void) get_X().anonymous_struct_member;
   (void) get_X().data_member;
   (void) get_X().static_data_member;
-  // expected-warning@-1 {{ignoring return value of function declared with 'nodiscard' attribute}}
   (void) get_X().unscoped_enum;
-  // expected-warning@-1 {{ignoring return value of function declared with 'nodiscard' attribute}}
   (void) get_X().scoped_enum;
-  // expected-warning@-1 {{ignoring return value of function declared with 'nodiscard' attribute}}
   (void) get_X().implicit_object_member_function();
   (void) get_X().static_member_function();
-  // expected-warning@-1 {{ignoring return value of function declared with 'nodiscard' attribute}}
+  (void) get_Ptr()->implicit_object_member_function();
+  (void) get_Ptr()->static_member_function();
 #if __cplusplus >= 202302L
   (void) get_X().explicit_object_member_function();
 #endif
diff --git a/clang/test/SemaCXX/ms-property.cpp b/clang/test/SemaCXX/ms-property.cpp
index d5799a8a4d363..f1424b9cb12bc 100644
--- a/clang/test/SemaCXX/ms-property.cpp
+++ b/clang/test/SemaCXX/ms-property.cpp
@@ -2,6 +2,7 @@
 // RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -emit-pch -o %t -verify %s
 // RUN: %clang_cc1 -triple=x86_64-pc-win32 -fms-compatibility -include-pch %t %s -ast-print -o - | FileCheck %s
 // RUN: %clang_cc1 -fdeclspec -fsyntax-only -verify %s -std=c++23
+// expected-no-diagnostics
 
 #ifndef HEADER
 #define HEADER
@@ -103,7 +104,6 @@ struct X {
 void f() {
   (void) get_x().imp;
   (void) get_x().st;
-  // expected-warning@-1 {{ignoring return value of function declared with 'nodiscard' attribute}}
 #if __cplusplus >= 202302L
   (void) get_x().exp;
 #endif

@cor3ntin cor3ntin requested review from shafik and erichkeane March 15, 2025 10:16
@cor3ntin cor3ntin changed the title [Clang] Do not emit nodiscard warnings for the base expr of static me… [Clang] Do not emit nodiscard warnings for the base expr of static member access Mar 15, 2025
@elbeno
Copy link

elbeno commented Mar 15, 2025

Thanks @cor3ntin, apologies for dupe.

@cor3ntin
Copy link
Contributor Author

@elbeno thanks for reporting anyway. I'm a bit worried that it was reported twice in a day... we are going to backport it asap!

Copy link
Collaborator

@shafik shafik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@cor3ntin cor3ntin merged commit 9a1e390 into llvm:main Mar 15, 2025
16 checks passed
@cor3ntin cor3ntin added this to the LLVM 20.X Release milestone Mar 15, 2025
@github-project-automation github-project-automation bot moved this to Needs Triage in LLVM Release Status Mar 15, 2025
@cor3ntin
Copy link
Contributor Author

/cherry-pick 9a1e390

@cor3ntin cor3ntin deleted the corentin/gh131410 branch March 15, 2025 21:29
@llvmbot
Copy link
Member

llvmbot commented Mar 15, 2025

Failed to create pull request for issue131450 https://github.com/llvm/llvm-project/actions/runs/13876789590

@llvmbot
Copy link
Member

llvmbot commented Mar 15, 2025

/pull-request #131474

@llvmbot llvmbot moved this from Needs Triage to Done in LLVM Release Status Mar 15, 2025
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
Development

Successfully merging this pull request may close these issues.

Clang 20 warns on nodiscard-call.static_member_function()
4 participants