[Clang] Do not warn on unused lifetime-extending vars with side effects...

[cor3ntin]

Summary

const auto & var = ObjectWithSideEffects();

Fixes https://github.com/llvm/llvm-project/issues/54489

[cor3ntin]

Cleanup test.

[aaron.ballman]

clang/lib/Sema/SemaDecl.cpp

1893–1894

1900

1907–1908

clang/test/SemaCXX/warn-unused-variables.cpp

275–278

I think the difference between Clang and GCC here is that GCC only diagnoses when this function is instantiated: https://godbolt.org/z/f6ddacT97

How do we behave when we instantiate it?

Also, what happens with the test case as-written in the bug report:

struct RAIIWrapper {
    RAIIWrapper();
    ~RAIIWrapper();
};

void foo() {
    auto const  guard = RAIIWrapper();
    auto const& guard2 = RAIIWrapper();
    auto && guard3 = RAIIWrapper();
}

[cor3ntin]

• Address Aaron's comments.
• Add more tests.

[aaron.ballman]

Thanks, the additional test coverage makes it more clear that the changes are good. Please add a release note when landing though!

[cor3ntin]

Thanks for the review Aaron <3

clang/docs/ReleaseNotes.rst

- ``-Wliteral-range`` will warn on floating-point equality comparisons with
   ``(float) f == 0.1`` is always false.
 - ``-Winline-namespace-reopened-noninline`` now takes into account that the
   ``inline`` keyword must appear on the original but not necessarily all
   extension definitions of an inline namespace and therefore points its note
   at the original definition. This fixes `Issue 50794 (PR51452)
   <https://github.com/llvm/llvm-project/issues/50794>`_.
 - ``-Wunused-but-set-variable`` now also warns if the variable is only used
   by unary operators.
+- ``-Wunused-variable`` no longer warn for references extending the lifetime
+  of temporaries with side effects. This fixes `Issue 54489
+  <https://github.com/llvm/llvm-project/issues/54489>`_.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 - Improve __builtin_dump_struct:
   - Support bitfields in struct and union.
   - Improve the dump format, dump both bitwidth(if its a bitfield) and field value.
   - Remove anonymous tag locations.
   - Beautify dump format, add indent for nested struct and struct members.

clang/lib/Sema/SemaDecl.cpp

static bool ShouldDiagnoseUnusedDecl(const NamedDecl *D) {
 
   // White-list anything that isn't a local variable.
   if (!isa<VarDecl>(D) || isa<ParmVarDecl>(D) || isa<ImplicitParamDecl>(D))
     return false;
 
   // Types of valid local variables should be complete, so this should succeed.
   if (const VarDecl *VD = dyn_cast<VarDecl>(D)) {
 
-    // White-list anything with an __attribute__((unused)) type.
+    const Expr *Init = VD->getInit();
+    if (const auto *Cleanups = dyn_cast_or_null<ExprWithCleanups>(Init))
+      Init = Cleanups->getSubExpr();
+
     const auto *Ty = VD->getType().getTypePtr();
 
     // Only look at the outermost level of typedef.
     if (const TypedefType *TT = Ty->getAs<TypedefType>()) {
+      // Allow anything marked with __attribute__((unused)).
       if (TT->getDecl()->hasAttr<UnusedAttr>())
         return false;
     }
 
+    // Warn for reference variables whose initializtion performs lifetime
+    // extension.
+    if (const auto *MTE = dyn_cast_or_null<MaterializeTemporaryExpr>(Init)) {
+      if (MTE->getExtendingDecl()) {
+        Ty = VD->getType().getNonReferenceType().getTypePtr();
+        Init = MTE->getSubExpr()->IgnoreImplicitAsWritten();
+      }
+    }
+
     // If we failed to complete the type for some reason, or if the type is
     // dependent, don't diagnose the variable.
     if (Ty->isIncompleteType() || Ty->isDependentType())
       return false;
 
     // Look at the element type to ensure that the warning behaviour is
     // consistent for both scalars and arrays.
     Ty = Ty->getBaseElementTypeUnsafe();
 
     if (const TagType *TT = Ty->getAs<TagType>()) {
       const TagDecl *Tag = TT->getDecl();
       if (Tag->hasAttr<UnusedAttr>())
         return false;
 
       if (const CXXRecordDecl *RD = dyn_cast<CXXRecordDecl>(Tag)) {
         if (!RD->hasTrivialDestructor() && !RD->hasAttr<WarnUnusedAttr>())
           return false;
 
-        if (const Expr *Init = VD->getInit()) {
-          if (const ExprWithCleanups *Cleanups =
-                  dyn_cast<ExprWithCleanups>(Init))
-            Init = Cleanups->getSubExpr();
+        if (Init) {
           const CXXConstructExpr *Construct =
             dyn_cast<CXXConstructExpr>(Init);
           if (Construct && !Construct->isElidable()) {
             CXXConstructorDecl *CD = Construct->getConstructor();
             if (!CD->isTrivial() && !RD->hasAttr<WarnUnusedAttr>() &&
                 (VD->getInit()->isValueDependent() || !VD->evaluateValue()))
               return false;
           }
 
           // Suppress the warning if we don't know how this is constructed, and
           // it could possibly be non-trivial constructor.
-          if (Init->isTypeDependent())
+          if (Init->isTypeDependent()) {
             for (const CXXConstructorDecl *Ctor : RD->ctors())
               if (!Ctor->isTrivial())
                 return false;
           }
+
+          // Suppress the warning if the constructor is unresolved because
+          // its arguments are dependent.
+          if (isa<CXXUnresolvedConstructExpr>(Init))
+            return false;
+        }
       }
     }
 
     // TODO: __attribute__((unused)) templates?
   }
 
   return true;
 }

clang/test/SemaCXX/warn-unused-variables.cpp

namespace ctor_with_cleanups {
   };
   void func() {
     S2 s((S1()));
   }
 }
 
 #include "Inputs/warn-unused-variables.h"
 
-namespace arrayRecords {
-
 class NonTriviallyDestructible {
 public:
   ~NonTriviallyDestructible() {}
 };
 
+namespace arrayRecords {
+
 struct Foo {
   int x;
   Foo(int x) : x(x) {}
 };
 
 struct Elidable {
   Elidable();
 };
void bar() {
   NonTriviallyDestructible array[N]; // no warning
 }
 
 void test() {
   foo(10);
   bar<2>();
 }
 
-}
+} // namespace arrayRecords
 
 #if __cplusplus >= 201103L
 namespace with_constexpr {
 template <typename T>
 struct Literal {
   T i;
   Literal() = default;
   constexpr Literal(T i) : i(i) {}

 };
 
 template <typename T>
 void foo(T &t) {
   S s{t};
 }
 }
 #endif
+
+// Ensure we do not warn on lifetime extension
+namespace gh54489 {
+
+void f() {
+  const auto &a = NonTriviallyDestructible();
+  const auto &b = a; // expected-warning {{unused variable 'b'}}
+#if __cplusplus >= 201103L
+  const auto &&c = NonTriviallyDestructible();
+  auto &&d = c; // expected-warning {{unused variable 'd'}}
+#endif
+}
+
+struct S {
+  S() = default;
+  S(const S &) = default;
+  S(int);
+};
+
+template <typename T>
+void foo(T &t) {
+  const auto &extended = S{t};
+}
+
+void test_foo() {
+  int i;
+  foo(i);
+}
+
+struct RAIIWrapper {
+  RAIIWrapper();
+  ~RAIIWrapper();
+};
+
+void RAIIWrapperTest() {
+  auto const guard = RAIIWrapper();
+  auto const &guard2 = RAIIWrapper();
+  auto &&guard3 = RAIIWrapper();
+}
+
+} // namespace gh54489