Fix nodiscard for volatile references

[erichkeane]

Summary

As reported here https://bugs.llvm.org/show_bug.cgi?id=34988
[[nodiscard]] warnings were not being suppressed for
volatile-ref return values.

[aaron.ballman]

lib/AST/Expr.cpp

2302 ↗

(On Diff #119528)

Does this properly handle calls through function pointers (which usually have implicit casts)?

[[nodiscard]] volatile char &(*fp)();
void g() {
  (void)fp(); // No diagnostic here, either?
}

[erichkeane]

lib/AST/Expr.cpp

2302 ↗

(On Diff #119528)

I added this test, and it seems to work correctly. Patch with additional test incoming.

[erichkeane]

Added function-pointer test.

[aaron.ballman]

LGTM!

cfe/trunk/lib/AST/Expr.cpp

case CStyleCastExprClass: {
     // volatile lvalue.
     const CastExpr *CE = cast<CastExpr>(this);
     if (CE->getCastKind() == CK_ToVoid) {
       if (CE->getSubExpr()->isGLValue() &&
           CE->getSubExpr()->getType().isVolatileQualified()) {
         const DeclRefExpr *DRE =
             dyn_cast<DeclRefExpr>(CE->getSubExpr()->IgnoreParens());
         if (!(DRE && isa<VarDecl>(DRE->getDecl()) &&
-              cast<VarDecl>(DRE->getDecl())->hasLocalStorage())) {
+              cast<VarDecl>(DRE->getDecl())->hasLocalStorage()) &&
+            !isa<CallExpr>(CE->getSubExpr()->IgnoreParens())) {
           return CE->getSubExpr()->isUnusedResultAWarning(WarnE, Loc,
                                                           R1, R2, Ctx);
         }
       }
       return false;
     }
 
     // If this is a cast to a constructor conversion, check the operand.

cfe/trunk/test/CXX/dcl.dcl/dcl.attr/dcl.attr.nodiscard/p2.cpp

 // RUN: %clang_cc1 -fsyntax-only -std=c++17 -verify -Wc++17-extensions %s
 // RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify -DEXT -Wc++17-extensions %s
 
 struct [[nodiscard]] S {};
 S get_s();
 S& get_s_ref();
 
 enum [[nodiscard]] E {};
 E get_e();
 
 [[nodiscard]] int get_i();
+[[nodiscard]] volatile int &get_vi();
 
 void f() {
   get_s(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
   get_i(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+  get_vi(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
   get_e(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
 
   // Okay, warnings are not encouraged
   get_s_ref();
   (void)get_s();
   (void)get_i();
+  (void)get_vi();
   (void)get_e();
 }
 
+[[nodiscard]] volatile char &(*fp)();
+void g() {
+  fp(); // expected-warning {{ignoring return value of function declared with 'nodiscard' attribute}}
+
+  // OK, warning suppressed.
+  (void)fp();
+}
 #ifdef EXT
 // expected-warning@4 {{use of the 'nodiscard' attribute is a C++17 extension}}
 // expected-warning@8 {{use of the 'nodiscard' attribute is a C++17 extension}}
 // expected-warning@11 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// expected-warning@12 {{use of the 'nodiscard' attribute is a C++17 extension}}
+// expected-warning@28 {{use of the 'nodiscard' attribute is a C++17 extension}}
 #endif