[Clang][SemaCXX] Add unused warning for variables declared in condition expressions

[hazohelet]

Summary

This patch marks the declarations with initializations in condition expressions such as
if (int var = init) as unused and unreferenced so that -Wunused can warn on them.

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

[tbaeder]

clang/lib/CodeGen/CGExpr.cpp

2852 ↗

(On Diff #529824)

Question for other reviewers: If we want to keep this assertion, would it instead make sense to mark the decl as used after emitting the diagnostic, in https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDecl.cpp#L2154 ?

[hazohelet]

Ping

[shafik]

clang/lib/Sema/SemaExprCXX.cpp

4020–4021

[tbaeder]

clang/lib/Sema/SemaExprCXX.cpp

4020–4021

That seem a bit over the top given that it's the only boolean parameter.

[shafik]

clang/lib/Sema/SemaExprCXX.cpp

4020–4021

Yeah, in this case it is blindingly obvious but I do see many cases in the code where folks use bugprone-argument-comment for single bool parameters and I just use it everywhere for consistency sake.

[aaron.ballman]

This is a bit of an odd situation -- the condition variable *is* used (e.g., its value is loaded to determine whether to go into the if branch or the else branch), so we should not be marking it as unused in Sema::CheckConditionVariable(). Instead, I think we need to decide what constitutes "unused" in this case. Consider:

struct RAII {
  int &x;

  RAII(int &ref) : x(ref) {}
  ~RAII() { x = 0; }

  operator bool() const { return true; }
};

void func() {
  int i = 12;
  if (RAII name{i}) {
  }
}

I don't think name is unused in the same way that i is unused in if (int i = 1) {}. So I think we only want this to apply to scalars and not class types. I think the changes should be happening in Sema::ShouldDiagnoseUnusedDecl() (and you might need to thread the Scope object through to that call -- my naive thinking is that a variable declaration in a condition scope should be diagnosed as unused if it is used but not referenced, but I could be off-base).

clang/docs/ReleaseNotes.rst

130

[hazohelet]

In D152495#4481588, @aaron.ballman wrote:

This is a bit of an odd situation -- the condition variable *is* used (e.g., its value is loaded to determine whether to go into the if branch or the else branch), so we should not be marking it as unused in Sema::CheckConditionVariable(). Instead, I think we need to decide what constitutes "unused" in this case. Consider:

struct RAII {
  int &x;

  RAII(int &ref) : x(ref) {}
  ~RAII() { x = 0; }

  operator bool() const { return true; }
};

void func() {
  int i = 12;
  if (RAII name{i}) {
  }
}

I don't think name is unused in the same way that i is unused in if (int i = 1) {}. So I think we only want this to apply to scalars and not class types. I think the changes should be happening in Sema::ShouldDiagnoseUnusedDecl() (and you might need to thread the Scope object through to that call -- my naive thinking is that a variable declaration in a condition scope should be diagnosed as unused if it is used but not referenced, but I could be off-base).

Thank you for the insightful feedback!
I agree that we should not be emitting Wunused warnings in condition expressions when the declaration is of class type.

About the Scope object, if you mean ConditionVarScope flag by "condition scope", it seems to be used only for the condition of for-statements. (https://github.com/llvm/llvm-project/blob/f36b0169b8821e431644cb240f081538eb2b55ef/clang/lib/Parse/ParseExprCXX.cpp#L2045-L2057) Also it does not push new scope but modifies the flag of the current scope.
So it does not seem to be usable for our purpose here.

Instead, I think used-but-not-referenced variable declarations can be diagnosed as unused regardless of whether they are declared in condition expressions. This saves us from deleting the assertion from CodeGen and modifying AST tests.

[hazohelet]

• Stop marking condition variable declarations as unused and diagnose all VarDecl that is marked used but not referenced
• Added test not to warn when the declaration is not aggregate type
• NFC changes (comments, typo)

[aaron.ballman]

clang/lib/Sema/SemaExprCXX.cpp

4018–4021

isAggregateType() includes arrays and I think we still want to diagnose an unused array.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

5355

Which test case covers this change?

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

2

317–318

This should diagnose the decomposition declaration as being unused, but that already works today (so let's make sure we don't get duplicate diagnostics for this case): https://godbolt.org/z/6jP6sW1x6

An additional test case after this would be to add:

else if (auto [a, b] = (int[2]){ 1, 2 }; a)
  return;

which should get no diagnostics.

We should also have these tests for the other kinds of conditions (for, switch).

321

Can you also add coverage for switch?

[cor3ntin]

clang/lib/Sema/SemaExprCXX.cpp

4018–4021

Should it not be Condition->hasSideEffects() ?
Hopefully we want to be consistent with existing behavior https://godbolt.org/z/6abbPhn4G

[hazohelet]

clang/lib/Sema/SemaExprCXX.cpp

4018–4021

The condition here only applies to condition variables.
C++ does not allow array type in condition variable, correct? I think it's OK to use isAggregateType as well then.

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

5355

I wanted to suppress warnings against implicitly declared variables in range statements like __range2. I'll add a test for this.

[cor3ntin]

clang/lib/Sema/SemaExprCXX.cpp

4018–4021

After a chat with Aaron, I have a few questions: Do we call Scope::AddDecl - or PushOnScopeChains somewhere?
My intuition is that if the condition is added to a scope, and we call ConditionVar->setReferenced(false);, the all the diagnostic mechanism in DiagnoseUnusedDecl should be triggered. and it seen to be because we do get a diagnostic.
So neither if (!T->isAggregateType()) or any check should be needed there

Can you just remove that if statement (just set setReferenced(false)) and see if it works? Otherwise, i think we need to understand why it doesn't but trying to reimplement the logic of ShouldDiagnoseUnusedDeclseems fraught with unbounded complexity.

[hazohelet]

Address comments from Aaron and Corentin

• Call ConditionVar->setReferenced(false) without any checks
• Added some more tests

[hazohelet]

clang/lib/Sema/SemaExprCXX.cpp

4018–4021

It didn't break any tests locally, but when the record type does not have a user-provided dtor, it is diagnosed as unused, which seems not nice. (example written in the test trivial_dtor)

[cor3ntin]

This looks reasonable to me now.
I'll wait next week before approving it so other get a chance to look at it :)

clang/lib/Sema/SemaExprCXX.cpp

4018–4021

It's still a declaration that does nothing, so diagnosing let you remove it
https://compiler-explorer.com/z/a1GGs9cxa this is consistent with local variables. I like the simplicity of the change.

[hazohelet]

clang/lib/Sema/SemaExprCXX.cpp

4018–4021

Ah, I see. Thanks!

[tbaeder]

@cor3ntin It's next week :)

[cor3ntin]

LGTM, thanks. Make sure you rebase the changes to the release notes

[hazohelet]

I reverted this change because it broke sanitizer buildbots.
https://lab.llvm.org/buildbot/#/builders/19/builds/18369
The cause of the errors here are relatively clear and I can speculatively replace if (std::error_code EC = makeCanonical(Path)) with if (makeCanonical(Path))

https://lab.llvm.org/buildbot/#/builders/94/builds/15865
https://lab.llvm.org/buildbot/#/builders/240/builds/12947
These 2 are not clear for me, so I'll take time to find the cause.

[tbaeder]

The fuzzer timeouts fail all the time; I usually just assume it's unrelated.

[aaron.ballman]

In D152495#4563520, @hazohelet wrote:

I reverted this change because it broke sanitizer buildbots.
https://lab.llvm.org/buildbot/#/builders/19/builds/18369
The cause of the errors here are relatively clear and I can speculatively replace if (std::error_code EC = makeCanonical(Path)) with if (makeCanonical(Path))

That changes makes sense to me.

https://lab.llvm.org/buildbot/#/builders/94/builds/15865
https://lab.llvm.org/buildbot/#/builders/240/builds/12947
These 2 are not clear for me, so I'll take time to find the cause.

Neither of these are problems with your patch -- they're flaky tests in this case.

[hazohelet]

In D152495#4566634, @aaron.ballman wrote:

In D152495#4563520, @hazohelet wrote:

I reverted this change because it broke sanitizer buildbots.
https://lab.llvm.org/buildbot/#/builders/19/builds/18369
The cause of the errors here are relatively clear and I can speculatively replace if (std::error_code EC = makeCanonical(Path)) with if (makeCanonical(Path))

That changes makes sense to me.

https://lab.llvm.org/buildbot/#/builders/94/builds/15865
https://lab.llvm.org/buildbot/#/builders/240/builds/12947
These 2 are not clear for me, so I'll take time to find the cause.

Neither of these are problems with your patch -- they're flaky tests in this case.

Thanks! I'll reland this patch with the unused std::error_code fix. I'll revert again if the same CI keeps failing for a few hours.

[chapuni]

Would this cause many warnings in Clang/LLVM tree?
https://lab.llvm.org/buildbot/#/builders/36/builds/36560

I hope you to fix possible warnings at first.

[hazohelet]

In D152495#4587804, @chapuni wrote:

Would this cause many warnings in Clang/LLVM tree?
https://lab.llvm.org/buildbot/#/builders/36/builds/36560

I hope you to fix possible warnings at first.

Thanks. I'll revert this change.
I was thinking of pushing speculative fixes upstream, but the lines that need fixing are not only a few lines, so I now think it needs a review.

[hazohelet]

Fixed newly-issued warnings when trying check-clang,llvm,lld,compiler-rt,cxx,cxxabi with -Werror=unused-variable locally using local build of clang

[nikic]

Please split the warning fixes off into a separate patch.

[hazohelet]

Removed warning fixes that are now in D158016

[goncharov]

there is a number of unused vaiables in conditional loops that are firing now, I have submitted https://github.com/llvm/llvm-project/commit/74f4daef0412be33002bd4e24a30cb47d0187ecf but I suspect it's just a start.
How did you checked the project code for that?

[smeenai]

In D152495#4628028, @goncharov wrote:

there is a number of unused vaiables in conditional loops that are firing now, I have submitted https://github.com/llvm/llvm-project/commit/74f4daef0412be33002bd4e24a30cb47d0187ecf but I suspect it's just a start.
How did you checked the project code for that?

Per rG01b88dd66d9d52d3f531a7d490e18c549f36f445, unused dyn_cast results should turn into isa.

[cor3ntin]

It is used, but only in an assert. Seems like the right fix!

In D152495#4628778, @smeenai wrote:

In D152495#4628028, @goncharov wrote:

there is a number of unused vaiables in conditional loops that are firing now, I have submitted https://github.com/llvm/llvm-project/commit/74f4daef0412be33002bd4e24a30cb47d0187ecf but I suspect it's just a start.
How did you checked the project code for that?

Per rG01b88dd66d9d52d3f531a7d490e18c549f36f445, unused dyn_cast results should turn into isa.

[goncharov]

due to this change we have a enourmous number of new warnings, on the other hand -Wunused-variable is a valuable warning. I am not sure what is the policy and best practices for warnings but maybe there is a possiblity to make a transition period for enabling this type of warning separetely and to allow updating existing code?

[hans]

In D152495#4628785, @cor3ntin wrote:

It is used, but only in an assert. Seems like the right fix!

I suppose it is technically, but I'm not sure the fix reads like an improvement to me... I guess that's often the case with unused variables vs. asserts though.

In D152495#4628870, @goncharov wrote:

due to this change we have a enourmous number of new warnings, on the other hand -Wunused-variable is a valuable warning. I am not sure what is the policy and best practices for warnings but maybe there is a possiblity to make a transition period for enabling this type of warning separetely and to allow updating existing code?

The usual policy is to put new warnings behind new flags so users can disable them selectively. It gets trickier when it's existing warnings getting enhanced like this. Would it be possible to put this new functionality behind a flag?

[aaron.ballman]

In D152495#4628877, @hans wrote:

In D152495#4628785, @cor3ntin wrote:

It is used, but only in an assert. Seems like the right fix!

I suppose it is technically, but I'm not sure the fix reads like an improvement to me... I guess that's often the case with unused variables vs. asserts though.

In D152495#4628870, @goncharov wrote:

due to this change we have a enourmous number of new warnings, on the other hand -Wunused-variable is a valuable warning. I am not sure what is the policy and best practices for warnings but maybe there is a possiblity to make a transition period for enabling this type of warning separetely and to allow updating existing code?

The usual policy is to put new warnings behind new flags so users can disable them selectively. It gets trickier when it's existing warnings getting enhanced like this. Would it be possible to put this new functionality behind a flag?

That might not be a bad idea in this case -- perhaps -Wunused-condition-variable and group it under -Wunused-variable?

[smeenai]

In D152495#4628877, @hans wrote:

In D152495#4628870, @goncharov wrote:

due to this change we have a enourmous number of new warnings, on the other hand -Wunused-variable is a valuable warning. I am not sure what is the policy and best practices for warnings but maybe there is a possiblity to make a transition period for enabling this type of warning separetely and to allow updating existing code?

The usual policy is to put new warnings behind new flags so users can disable them selectively. It gets trickier when it's existing warnings getting enhanced like this. Would it be possible to put this new functionality behind a flag?

Yeah, this has been a recurring issue when existing warnings get enhanced. It's often not feasible to fix all new instances right away, but you don't want to disable or -Wno-error the warning either cos then new issues can start creeping in, leaving you with no good options. Maybe we should have a policy around even enhancements to existing warnings always going in their own subgroup, so that we can disable them selectively while not regressing anything? @aaron.ballman, what are your thoughts?

[hans]

In D152495#4628887, @aaron.ballman wrote:

That might not be a bad idea in this case -- perhaps -Wunused-condition-variable and group it under -Wunused-variable?

Sounds god to me conceptually. (I don't know the code, so don't know how hard it is practically.)

[MaskRay]

In D152495#4628903, @smeenai wrote:

In D152495#4628877, @hans wrote:

In D152495#4628870, @goncharov wrote:

due to this change we have a enourmous number of new warnings, on the other hand -Wunused-variable is a valuable warning. I am not sure what is the policy and best practices for warnings but maybe there is a possiblity to make a transition period for enabling this type of warning separetely and to allow updating existing code?

The usual policy is to put new warnings behind new flags so users can disable them selectively. It gets trickier when it's existing warnings getting enhanced like this. Would it be possible to put this new functionality behind a flag?

Yeah, this has been a recurring issue when existing warnings get enhanced. It's often not feasible to fix all new instances right away, but you don't want to disable or -Wno-error the warning either cos then new issues can start creeping in, leaving you with no good options.

(I haven't really read the technical side of this patch yet.)

Yes, from me observing how we internally treat such warnings.

Maybe we should have a policy around even enhancements to existing warnings always going in their own subgroup, so that we can disable them selectively while not regressing anything? @aaron.ballman, what are your thoughts?

Sometimes an improved diagnostic just triggers in very few new places that can be easily handled.
Sometimes there are many more places so that it is difficult to fix in a satisfactory timeline (as a rolling update user doesn't want to postpone releases a long time).

I think sometimes it's just difficult for contributors to figure out whether a diagnostic is going to require substantial cleanup or just a little effort. In this case rolling update users (like we) providing feedback is useful whether a new sub -Wxxx is needed.

[pranavk]

I agree -Wunused-condition-variable sounds like a good idea. There are still numerous instances of this warning/error showing up when doing a self-build of LLVM, let alone warnings/errors that are showing up in internal code bases who are using LLVM HEAD for compiling their codebases. -Wunused-condition-variable would make this transition smoother.

[nickdesaulniers]

Here's a more blatant regression caused by this patch.

https://godbolt.org/z/q19Ge64G3

Causing breakage for the Linux kernel:
https://github.com/ClangBuiltLinux/linux/issues/1926

[aaron.ballman]

In D152495#4631829, @nickdesaulniers wrote:

Here's a more blatant regression caused by this patch.

https://godbolt.org/z/q19Ge64G3

Causing breakage for the Linux kernel:
https://github.com/ClangBuiltLinux/linux/issues/1926

Because of the amount of disruption and the fact that we're trying to get 17.x out the door while transitioning to a new code review workflow, let's revert this for now. The __attribute__((used)) problem seems to be something deeper, as [[maybe_unused]] does work properly (https://godbolt.org/z/e7YK1G69e), and putting this diagnostic under its own warning group will help folks adjust to the new behavior.

[hazohelet]

Thank you for the feedback and the revert!

Distinguishing condition variables from other unused variables in Wunused warnings was something I was trying initially, and I agree that it's the best way forward.
The difficulty is in how to know whether an unused VarDecl is a condition variable or not in the phase of emitting Wunused-variable. We may need to add a boolean flag IsConditionVar in VarDecl or something.

The reason [[maybe_unused]] works well is that it is explicitly handled in Sema::ShouldDiagnoseUnusedDecl (https://github.com/llvm/llvm-project/blob/e7bd43675753476e97e63aa13c13b3498407ed1c/clang/lib/Sema/SemaDecl.cpp#L2005-L2007)

The used attribute regression was caused by the "used but not-referenced variables should be diagnosed as unused" change. Once we have some way of knowing that a VarDecl is a condition variable, we should only apply this change to condition variables, thereby avoiding regression.

clang/docs/ReleaseNotes.rst

 -------------------------
 
 Attribute Changes in Clang
 --------------------------
 
 Improvements to Clang's diagnostics
 -----------------------------------
 - Clang constexpr evaluator now prints template arguments when displaying
   template-specialization function calls.
 - Clang contexpr evaluator now displays notes as well as an error when a constructor
   of a base class is not called in the constructor of its derived class.
 - Clang no longer emits ``-Wmissing-variable-declarations`` for variables declared
   with the ``register`` storage class.
+- Clang now warns on unused variables declared and initialized in condition
+  expressions.
+  (`#61681: <https://github.com/llvm/llvm-project/issues/61681>`_)
 
 Bug Fixes in This Version
 -------------------------
 - Fixed an issue where a class template specialization whose declaration is
   instantiated in one module and whose definition is instantiated in another
   module may end up with members associated with the wrong declaration of the
   class, which can result in miscompiles in some cases.
 - Fix crash on use of a variadic overloaded operator.

clang/include/clang/ASTMatchers/ASTMatchers.h

 ///   +(void)foo;
 ///   @end
 ///   ...
 ///   [I foo]
 /// \endcode
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
     callee, AST_POLYMORPHIC_SUPPORTED_TYPES(ObjCMessageExpr, CallExpr),
     internal::Matcher<Decl>, InnerMatcher, 1) {
-  if (const auto *CallNode = dyn_cast<CallExpr>(&Node))
+  if (isa<CallExpr>(&Node))
     return callExpr(hasDeclaration(InnerMatcher))
         .matches(Node, Finder, Builder);
   else {
     // The dynamic cast below is guaranteed to succeed as there are only 2
     // supported return types.
     const auto *MsgNode = cast<ObjCMessageExpr>(&Node);
     const Decl *DeclNode = MsgNode->getMethodDecl();
     return (DeclNode != nullptr &&

clang/lib/AST/Interp/ByteCodeStmtGen.cpp

bool ByteCodeStmtGen<Emitter>::visitIfStmt(const IfStmt *IS) {
   BlockScope<Emitter> IfScope(this);
 
   if (IS->isNonNegatedConsteval())
     return visitStmt(IS->getThen());
   if (IS->isNegatedConsteval())
     return IS->getElse() ? visitStmt(IS->getElse()) : true;
 
   if (auto *CondInit = IS->getInit())
-    if (!visitStmt(IS->getInit()))
+    if (!visitStmt(CondInit))
       return false;
 
   if (const DeclStmt *CondDecl = IS->getConditionVariableDeclStmt())
     if (!visitDeclStmt(CondDecl))
       return false;
 
   if (!this->visitBool(IS->getCond()))
     return false;

clang/lib/CodeGen/BackendUtil.cpp

 void clang::EmbedObject(llvm::Module *M, const CodeGenOptions &CGOpts,
                         DiagnosticsEngine &Diags) {
   if (CGOpts.OffloadObjects.empty())
     return;
 
   for (StringRef OffloadObject : CGOpts.OffloadObjects) {
     llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> ObjectOrErr =
         llvm::MemoryBuffer::getFileOrSTDIN(OffloadObject);
-    if (std::error_code EC = ObjectOrErr.getError()) {
+    if (ObjectOrErr.getError()) {
       auto DiagID = Diags.getCustomDiagID(DiagnosticsEngine::Error,
                                           "could not open '%0' for embedding");
       Diags.Report(DiagID) << OffloadObject;
       return;
     }
 
     llvm::embedBufferInModule(*M, **ObjectOrErr, ".llvm.offloading",
                               Align(object::OffloadBinary::getAlignment()));
   }
 }

clang/lib/CodeGen/CGExprConstant.cpp

llvm::Constant *VisitCastExpr(CastExpr *E, QualType destType) {
     case CK_ConstructorConversion:
       return Visit(subExpr, destType);
 
     case CK_ArrayToPointerDecay:
       if (const auto *S = dyn_cast<StringLiteral>(subExpr))
         return CGM.GetAddrOfConstantStringFromLiteral(S).getPointer();
       return nullptr;
     case CK_NullToPointer:
-      if (llvm::Constant *C = Visit(subExpr, destType))
+      if (Visit(subExpr, destType))
         return CGM.EmitNullConstant(destType);
       return nullptr;
 
     case CK_IntToOCLSampler:
       llvm_unreachable("global sampler variables are not generated");
 
     case CK_IntegralCast: {
       QualType FromType = subExpr->getType();

clang/lib/CodeGen/CGOpenMPRuntime.cpp

static llvm::TargetRegionEntryInfo getEntryInfoFromPresumedLoc(
     CodeGenModule &CGM, llvm::OpenMPIRBuilder &OMPBuilder,
     SourceLocation BeginLoc, llvm::StringRef ParentName = "") {
 
   auto FileInfoCallBack = [&]() {
     SourceManager &SM = CGM.getContext().getSourceManager();
     PresumedLoc PLoc = SM.getPresumedLoc(BeginLoc);
 
     llvm::sys::fs::UniqueID ID;
-    if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID)) {
+    if (llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID)) {
       PLoc = SM.getPresumedLoc(BeginLoc, /*UseLineDirectives=*/false);
     }
 
     return std::pair<std::string, uint64_t>(PLoc.getFilename(), PLoc.getLine());
   };
 
   return OMPBuilder.getTargetEntryUniqueInfo(FileInfoCallBack, ParentName);
 }

clang/lib/CodeGen/CodeGenModule.cpp

if (getLangOpts().CUID.empty()) {
     llvm::MD5 Hash;
     llvm::MD5::MD5Result Result;
     for (const auto &Arg : PreprocessorOpts.Macros)
       Hash.update(Arg.first);
     Hash.final(Result);
 
     // Get the UniqueID for the file containing the decl.
     llvm::sys::fs::UniqueID ID;
-    if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID)) {
+    if (llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID)) {
       PLoc = SM.getPresumedLoc(D->getLocation(), /*UseLineDirectives=*/false);
       assert(PLoc.isValid() && "Source location is expected to be valid.");
       if (auto EC = llvm::sys::fs::getUniqueID(PLoc.getFilename(), ID))
         SM.getDiagnostics().Report(diag::err_cannot_open_file)
             << PLoc.getFilename() << EC.message();
     }
     OS << llvm::format("%x", ID.getFile()) << llvm::format("%x", ID.getDevice())
        << "_" << llvm::utohexstr(Result.low(), /*LowerCase=*/true, /*Width=*/8);

clang/lib/Driver/ToolChains/Cuda.cpp

for (const auto &II : Inputs) {
     if (llvm::sys::path::extension(InputFile) != ".cubin") {
       // If there are no actions above this one then this is direct input and we
       // can copy it. Otherwise the input is internal so a `.cubin` file should
       // exist.
       if (II.getAction() && II.getAction()->getInputs().size() == 0) {
         const char *CubinF =
             Args.MakeArgString(getToolChain().getDriver().GetTemporaryPath(
                 llvm::sys::path::stem(InputFile), "cubin"));
-        if (std::error_code EC =
-                llvm::sys::fs::copy_file(InputFile, C.addTempFile(CubinF)))
+        if (llvm::sys::fs::copy_file(InputFile, C.addTempFile(CubinF)))
           continue;
 
         CmdArgs.push_back(CubinF);
       } else {
         SmallString<256> Filename(InputFile);
         llvm::sys::path::replace_extension(Filename, "cubin");
         CmdArgs.push_back(Args.MakeArgString(Filename));
       }

clang/lib/Sema/SemaDecl.cpp

for (auto *BD : DD->bindings()) {
       if (BD->isReferenced())
         return false;
       IsAllPlaceholders = IsAllPlaceholders && BD->isPlaceholderVar(LangOpts);
     }
     if (IsAllPlaceholders)
       return false;
   } else if (!D->getDeclName()) {
     return false;
-  } else if (D->isReferenced() || D->isUsed()) {
+  } else if (D->isReferenced() || (!isa<VarDecl>(D) && D->isUsed())) {
     return false;
   }
 
   if (D->isPlaceholderVar(LangOpts))
     return false;
 
   if (D->hasAttr<UnusedAttr>() || D->hasAttr<ObjCPreciseLifetimeAttr>() ||
       D->hasAttr<CleanupAttr>())

clang/lib/Sema/SemaExprCXX.cpp

else if (T->isArrayType())
     return ExprError(Diag(ConditionVar->getLocation(),
                           diag::err_invalid_use_of_array_type)
                      << ConditionVar->getSourceRange());
 
   ExprResult Condition = BuildDeclRefExpr(
       ConditionVar, ConditionVar->getType().getNonReferenceType(), VK_LValue,
       ConditionVar->getLocation());
 
+  // Ensure that `-Wunused-variable` will be emitted for condition variables
+  // that are not referenced later. e.g.: if (int var = init());
+  ConditionVar->setReferenced(/*R=*/false);
+
   switch (CK) {
   case ConditionKind::Boolean:
     return CheckBooleanCondition(StmtLoc, Condition.get());
 
   case ConditionKind::ConstexprIf:
     return CheckBooleanCondition(StmtLoc, Condition.get(), true);
 
   case ConditionKind::Switch:

clang/lib/Sema/SemaTemplateInstantiateDecl.cpp

void Sema::BuildVariableInstantiation(
   } else {
     InstantiateVariableInitializer(NewVar, OldVar, TemplateArgs);
   }
 
   // Diagnose unused local variables with dependent types, where the diagnostic
   // will have been deferred.
   if (!NewVar->isInvalidDecl() &&
       NewVar->getDeclContext()->isFunctionOrMethod() &&
-      OldVar->getType()->isDependentType())
+      OldVar->getType()->isDependentType() && !OldVar->isImplicit())
     DiagnoseUnusedDecl(NewVar);
 }
 
 /// Instantiate the initializer of a variable.
 void Sema::InstantiateVariableInitializer(
     VarDecl *Var, VarDecl *OldVar,
     const MultiLevelTemplateArgumentList &TemplateArgs) {
   if (ASTMutationListener *L = getASTContext().getASTMutationListener())

clang/lib/StaticAnalyzer/Checkers/FuchsiaHandleChecker.cpp

if (const auto *TypeDefTy = FuncDecl->getReturnType()->getAs<TypedefType>())
     if (TypeDefTy->getDecl()->getName() == ErrorTypeName)
       ResultSymbol = Call.getReturnValue().getAsSymbol();
 
   // Function returns an open handle.
   if (hasFuchsiaAttr<AcquireHandleAttr>(FuncDecl)) {
     SymbolRef RetSym = Call.getReturnValue().getAsSymbol();
     Notes.push_back([RetSym, FuncDecl](BugReport &BR) -> std::string {
       auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
-      if (auto IsInteresting = PathBR->getInterestingnessKind(RetSym)) {
+      if (PathBR->getInterestingnessKind(RetSym)) {
         std::string SBuf;
         llvm::raw_string_ostream OS(SBuf);
         OS << "Function '" << FuncDecl->getDeclName()
            << "' returns an open handle";
         return SBuf;
       } else
         return "";
     });
     State =
         State->set<HStateMap>(RetSym, HandleState::getMaybeAllocated(nullptr));
   } else if (hasFuchsiaUnownedAttr<AcquireHandleAttr>(FuncDecl)) {
     // Function returns an unowned handle
     SymbolRef RetSym = Call.getReturnValue().getAsSymbol();
     Notes.push_back([RetSym, FuncDecl](BugReport &BR) -> std::string {
       auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
-      if (auto IsInteresting = PathBR->getInterestingnessKind(RetSym)) {
+      if (PathBR->getInterestingnessKind(RetSym)) {
         std::string SBuf;
         llvm::raw_string_ostream OS(SBuf);
         OS << "Function '" << FuncDecl->getDeclName()
            << "' returns an unowned handle";
         return SBuf;
       } else
         return "";
     });
for (SymbolRef Handle : Handles) {
           reportDoubleRelease(Handle, Call.getArgSourceRange(Arg), C);
           return;
         } else if (HState && HState->isUnowned()) {
           reportUnownedRelease(Handle, Call.getArgSourceRange(Arg), C);
           return;
         } else {
           Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
             auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
-            if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
+            if (PathBR->getInterestingnessKind(Handle)) {
               std::string SBuf;
               llvm::raw_string_ostream OS(SBuf);
               OS << "Handle released through " << ParamDiagIdx
                  << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
               return SBuf;
             } else
               return "";
           });
           State = State->set<HStateMap>(Handle, HandleState::getReleased());
         }
       } else if (hasFuchsiaAttr<AcquireHandleAttr>(PVD)) {
         Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
           auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
-          if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
+          if (PathBR->getInterestingnessKind(Handle)) {
             std::string SBuf;
             llvm::raw_string_ostream OS(SBuf);
             OS << "Handle allocated through " << ParamDiagIdx
                << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
             return SBuf;
           } else
             return "";
         });
         State = State->set<HStateMap>(
             Handle, HandleState::getMaybeAllocated(ResultSymbol));
       } else if (hasFuchsiaUnownedAttr<AcquireHandleAttr>(PVD)) {
         Notes.push_back([Handle, ParamDiagIdx](BugReport &BR) -> std::string {
           auto *PathBR = static_cast<PathSensitiveBugReport *>(&BR);
-          if (auto IsInteresting = PathBR->getInterestingnessKind(Handle)) {
+          if (PathBR->getInterestingnessKind(Handle)) {
             std::string SBuf;
             llvm::raw_string_ostream OS(SBuf);
             OS << "Unowned handle allocated through " << ParamDiagIdx
                << llvm::getOrdinalSuffix(ParamDiagIdx) << " parameter";
             return SBuf;
           } else
             return "";
         });

clang/lib/StaticAnalyzer/Checkers/Yaml.h

std::optional<T> getConfiguration(CheckerManager &Mgr, Checker *Chk,
                                   StringRef Option, StringRef ConfigFile) {
   if (ConfigFile.trim().empty())
     return std::nullopt;
 
   llvm::vfs::FileSystem *FS = llvm::vfs::getRealFileSystem().get();
   llvm::ErrorOr<std::unique_ptr<llvm::MemoryBuffer>> Buffer =
       FS->getBufferForFile(ConfigFile.str());
 
-  if (std::error_code ec = Buffer.getError()) {
+  if (Buffer.getError()) {
     Mgr.reportInvalidCheckerOptionValue(Chk, Option,
                                         "a valid filename instead of '" +
                                             std::string(ConfigFile) + "'");
     return std::nullopt;
   }
 
   llvm::yaml::Input Input(Buffer.get()->getBuffer());
   T Config;

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

 // RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify %s
-// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++1y-extensions -verify -std=c++11 %s
+// RUN: %clang_cc1 -fsyntax-only -Wunused-variable -Wunused-label -Wno-c++14-extensions -Wno-c++17-extensions -verify -std=c++11 %s
 template<typename T> void f() {
   T t;
   t = 17;
 }
 
 // PR5407
 struct A { A(); };
 struct B { ~B(); };

 
 void RAIIWrapperTest() {
   auto const guard = RAIIWrapper();
   auto const &guard2 = RAIIWrapper();
   auto &&guard3 = RAIIWrapper();
 }
 
 } // namespace gh54489
+
+namespace inside_condition {
+  void ifs() {
+    if (int hoge = 0) // expected-warning {{unused variable 'hoge'}}
+      return;
+    if (const int const_hoge = 0) // expected-warning {{unused variable 'const_hoge'}}
+      return;
+    else if (int fuga = 0)
+      (void)fuga;
+    else if (int used = 1; int catched = used) // expected-warning {{unused variable 'catched'}}
+      return;
+    else if (int refed = 1; int used = refed)
+      (void)used;
+    else if (int unused1 = 2; int unused2 = 3) // expected-warning {{unused variable 'unused1'}} \
+                                               // expected-warning {{unused variable 'unused2'}}
+      return;
+    else if (int unused = 4; int used = 5) // expected-warning {{unused variable 'unused'}}
+      (void)used;
+    else if (int used = 6; int unused = 7) // expected-warning {{unused variable 'unused'}}
+      (void)used;
+    else if (int used1 = 8; int used2 = 9)
+      (void)(used1 + used2);
+    else if (auto [a, b] = (int[2]){ 1, 2 }; 1) // expected-warning {{unused variable '[a, b]'}}
+      return;
+    else if (auto [a, b] = (int[2]){ 1, 2 }; a)
+      return;
+  }
+
+  void fors() {
+    for (int i = 0;int unused = 0;); // expected-warning {{unused variable 'i'}} \
+                                     // expected-warning {{unused variable 'unused'}}
+    for (int i = 0;int used = 0;) // expected-warning {{unused variable 'i'}}
+      (void)used;
+      while(int var = 1) // expected-warning {{unused variable 'var'}}
+        return;
+  }
+
+  void whiles() {
+    while(int unused = 1) // expected-warning {{unused variable 'unused'}}
+      return;
+    while(int used = 1)
+      (void)used;
+  }
+
+
+  void switches() {
+    switch(int unused = 1) { // expected-warning {{unused variable 'unused'}}
+      case 1: return;
+    }
+    switch(constexpr int used = 3; int unused = 4) { // expected-warning {{unused variable 'unused'}}
+      case used: return;
+    }
+    switch(int used = 3; int unused = 4) { // expected-warning {{unused variable 'unused'}}
+      case 3: (void)used;
+    }
+    switch(constexpr int used1 = 0; constexpr int used2 = 6) {
+      case (used1+used2): return;
+    }
+    switch(auto [a, b] = (int[2]){ 1, 2 }; 1) { // expected-warning {{unused variable '[a, b]'}}
+      case 1: return;
+    }
+    switch(auto [a, b] = (int[2]){ 1, 2 }; b) {
+      case 1: return;
+    }
+    switch(auto [a, b] = (int[2]){ 1, 2 }; 1) {
+      case 1: (void)a;
+    }
+  }
+  template <typename T>
+  struct Vector {
+    void doIt() {
+      for (auto& e : elements){} // expected-warning {{unused variable 'e'}}
+    }
+    T elements[10];
+  };
+  void ranged_for() {
+    Vector<int>    vector;
+    vector.doIt(); // expected-note {{here}}
+  }
+
+
+  struct RAII {
+    int &x;
+    RAII(int &ref) : x(ref) {}
+    ~RAII() { x = 0;}
+    operator int() const { return 1; }
+  };
+  void aggregate() {
+    int x = 10;
+    int y = 10;
+    if (RAII var = x) {}
+    for(RAII var = x; RAII var2 = y;) {}
+    while (RAII var = x) {}
+    switch (RAII var = x) {}
+  }
+
+  struct TrivialDtor{
+    int &x;
+    TrivialDtor(int &ref) : x(ref) { ref = 32; }
+    operator int() const { return 1; }
+  };
+  void trivial_dtor() {
+    int x = 10;
+    int y = 10;
+    if (TrivialDtor var = x) {} // expected-warning {{unused variable 'var'}}
+    for(TrivialDtor var = x; TrivialDtor var2 = y;) {} // expected-warning {{unused variable 'var'}} \
+                                         // expected-warning {{unused variable 'var2'}}
+    while (TrivialDtor var = x) {} // expected-warning {{unused variable 'var'}}
+    switch (TrivialDtor var = x) {} // expected-warning {{unused variable 'var'}}
+  }
+
+} // namespace inside_condition

clang/tools/clang-scan-deps/ClangScanDeps.cpp

llvm::sys::fs::createTemporaryFile("print-resource-dir-error",
                                        "" /*no-suffix*/, ErrorFile);
     llvm::FileRemover OutputRemover(OutputFile.c_str());
     llvm::FileRemover ErrorRemover(ErrorFile.c_str());
     std::optional<StringRef> Redirects[] = {
         {""}, // Stdin
         OutputFile.str(),
         ErrorFile.str(),
     };
-    if (const int RC = llvm::sys::ExecuteAndWait(
-            ClangBinaryPath, PrintResourceDirArgs, {}, Redirects)) {
+    if (llvm::sys::ExecuteAndWait(ClangBinaryPath, PrintResourceDirArgs, {},
+                                  Redirects)) {
       auto ErrorBuf = llvm::MemoryBuffer::getFile(ErrorFile.c_str());
       llvm::errs() << ErrorBuf.get()->getBuffer();
       return "";
     }
 
     auto OutputBuf = llvm::MemoryBuffer::getFile(OutputFile.c_str());
     if (!OutputBuf)
       return "";

lld/MachO/Driver.cpp

if (entry == loadedArchives.end()) {
         return file;
     }
 
     bool isLCLinkerForceLoad = loadType == LoadType::LCLinkerOption &&
                                config->forceLoadSwift &&
                                path::filename(path).starts_with("libswift");
     if ((isCommandLineLoad && config->allLoad) ||
         loadType == LoadType::CommandLineForce || isLCLinkerForceLoad) {
-      if (std::optional<MemoryBufferRef> buffer = readFile(path)) {
+      if (readFile(path)) {
         Error e = Error::success();
         for (const object::Archive::Child &c : file->getArchive().children(e)) {
           StringRef reason;
           switch (loadType) {
             case LoadType::LCLinkerOption:
               reason = "LC_LINKER_OPTION";
               break;
             case LoadType::CommandLineForce:
if ((isCommandLineLoad && config->allLoad) ||
       }
     } else if (isCommandLineLoad && config->forceLoadObjC) {
       for (const object::Archive::Symbol &sym : file->getArchive().symbols())
         if (sym.getName().starts_with(objc::klass))
           file->fetch(sym);
 
       // TODO: no need to look for ObjC sections for a given archive member if
       // we already found that it contains an ObjC symbol.
-      if (std::optional<MemoryBufferRef> buffer = readFile(path)) {
+      if (readFile(path)) {
         Error e = Error::success();
         for (const object::Archive::Child &c : file->getArchive().children(e)) {
           Expected<MemoryBufferRef> mb = c.getMemoryBufferRef();
           if (!mb || !hasObjCSection(*mb))
             continue;
           if (Error e = file->fetch(c, "-ObjC"))
             error(toString(file) + ": -ObjC failed to load archive member: " +
                   toString(std::move(e)));

llvm/lib/Bitcode/Reader/MetadataLoader.cpp

#endif
   /// DISubprogram's retainedNodes.
   void upgradeCULocals() {
     if (NamedMDNode *CUNodes = TheModule.getNamedMetadata("llvm.dbg.cu")) {
       for (unsigned I = 0, E = CUNodes->getNumOperands(); I != E; ++I) {
         auto *CU = dyn_cast<DICompileUnit>(CUNodes->getOperand(I));
         if (!CU)
           continue;
 
-        if (auto *RawImported = CU->getRawImportedEntities()) {
+        if (CU->getRawImportedEntities()) {
           // Collect a set of imported entities to be moved.
           SetVector<Metadata *> EntitiesToRemove;
           for (Metadata *Op : CU->getImportedEntities()->operands()) {
             auto *IE = cast<DIImportedEntity>(Op);
-            if (auto *S = dyn_cast_or_null<DILocalScope>(IE->getScope())) {
+            if (isa_and_present<DILocalScope>(IE->getScope())) {
               EntitiesToRemove.insert(IE);
             }
           }
 
           if (!EntitiesToRemove.empty()) {
             // Make a new list of CU's 'imports'.
             SmallVector<Metadata *> NewImports;
             for (Metadata *Op : CU->getImportedEntities()->operands()) {

llvm/lib/DebugInfo/DWARF/DWARFContext.cpp

DWARFCompileUnit *DWARFContext::getCompileUnitForDataAddress(uint64_t Address) {
   //      put the globals in the aranges, but this isn't true for gcc.
   //   2. Even if the global variable is in a .debug_arange, global variables
   //      may not be captured in the [start, end) addresses described by the
   //      parent compile unit.
   //
   // So, we walk the CU's and their child DI's manually, looking for the
   // specific global variable.
   for (std::unique_ptr<DWARFUnit> &CU : compile_units()) {
-    if (DWARFDie Die = CU->getVariableForAddress(Address)) {
+    if (CU->getVariableForAddress(Address)) {
       return static_cast<DWARFCompileUnit *>(CU.get());
     }
   }
   return nullptr;
 }
 
 DWARFContext::DIEsForAddress DWARFContext::getDIEsForAddress(uint64_t Address) {
   DIEsForAddress Result;

llvm/lib/DebugInfo/DWARF/DWARFUnit.cpp

for (const DWARFLocationExpression &Location : *Locations) {
     Address = LocationAddr;
     break;
   }
 
   // Get the size of the global variable. If all else fails (i.e. the global has
   // no type), then we use a size of one to still allow symbolization of the
   // exact address.
   uint64_t GVSize = 1;
-  if (DWARFDie BaseType = Die.getAttributeValueAsReferencedDie(DW_AT_type))
+  if (Die.getAttributeValueAsReferencedDie(DW_AT_type))
     if (std::optional<uint64_t> Size = Die.getTypeSize(getAddressByteSize()))
       GVSize = *Size;
 
   if (Address != UINT64_MAX)
     VariableDieMap[Address] = {Address + GVSize, Die};
 }
 
 DWARFDie DWARFUnit::getVariableForAddress(uint64_t Address) {

llvm/lib/DebugInfo/GSYM/DwarfTransformer.cpp

void DwarfTransformer::handleDie(raw_ostream *OS, CUInfo &CUI, DWARFDie Die) {
   for (DWARFDie ChildDie : Die.children())
     handleDie(OS, CUI, ChildDie);
 }
 
 Error DwarfTransformer::convert(uint32_t NumThreads, raw_ostream *OS) {
   size_t NumBefore = Gsym.getNumFunctionInfos();
   auto getDie = [&](DWARFUnit &DwarfUnit) -> DWARFDie {
     DWARFDie ReturnDie = DwarfUnit.getUnitDIE(false);
-    if (std::optional<uint64_t> DWOId = DwarfUnit.getDWOId()) {
+    if (DwarfUnit.getDWOId()) {
       DWARFUnit *DWOCU = DwarfUnit.getNonSkeletonUnitDIE(false).getDwarfUnit();
       if (OS && !DWOCU->isDWOUnit()) {
         std::string DWOName = dwarf::toString(
             DwarfUnit.getUnitDIE().find(
                 {dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}),
             "");
         *OS << "warning: Unable to retrieve DWO .debug_info section for "
             << DWOName << "\n";

llvm/lib/IR/PrintPasses.cpp

 std::string llvm::doSystemDiff(StringRef Before, StringRef After,
                                StringRef OldLineFormat, StringRef NewLineFormat,
                                StringRef UnchangedLineFormat) {
   // Store the 2 bodies into temporary files and call diff on them
   // to get the body of the node.
   static SmallVector<int> FD{-1, -1, -1};
   SmallVector<StringRef> SR{Before, After};
   static SmallVector<std::string> FileName{"", "", ""};
-  if (auto Err = prepareTempFiles(FD, SR, FileName))
+  if (prepareTempFiles(FD, SR, FileName))
     return "Unable to create temporary file.";
 
   static ErrorOr<std::string> DiffExe = sys::findProgramByName(DiffBinary);
   if (!DiffExe)
     return "Unable to find diff executable.";
 
   SmallString<128> OLF, NLF, ULF;
   ("--old-line-format=" + OldLineFormat).toVector(OLF);
if (Result < 0)
     return "Error executing system diff.";
   std::string Diff;
   auto B = MemoryBuffer::getFile(FileName[2]);
   if (B && *B)
     Diff = (*B)->getBuffer().str();
   else
     return "Unable to read result.";
 
-  if (auto Err = cleanUpTempFiles(FileName))
+  if (cleanUpTempFiles(FileName))
     return "Unable to remove temporary file.";
 
   return Diff;
 }

llvm/lib/Passes/StandardInstrumentations.cpp

if (TestChanged != "")
     TextChangeReporter<std::string>::registerRequiredCallbacks(PIC);
 }
 
 void IRChangedTester::handleIR(const std::string &S, StringRef PassID) {
   // Store the body into a temporary file
   static SmallVector<int> FD{-1};
   SmallVector<StringRef> SR{S};
   static SmallVector<std::string> FileName{""};
-  if (auto Err = prepareTempFiles(FD, SR, FileName)) {
+  if (prepareTempFiles(FD, SR, FileName)) {
     dbgs() << "Unable to create temporary file.";
     return;
   }
   static ErrorOr<std::string> Exe = sys::findProgramByName(TestChanged);
   if (!Exe) {
     dbgs() << "Unable to find test-changed executable.";
     return;
   }
 
   StringRef Args[] = {TestChanged, FileName[0], PassID};
   int Result = sys::ExecuteAndWait(*Exe, Args);
   if (Result < 0) {
     dbgs() << "Error executing test-changed executable.";
     return;
   }
 
-  if (auto Err = cleanUpTempFiles(FileName))
+  if (cleanUpTempFiles(FileName))
     dbgs() << "Unable to remove temporary file.";
 }
 
 void IRChangedTester::handleInitialIR(Any IR) {
   // Always test the initial module.
   // Unwrap and print directly to avoid filtering problems in general routines.
   std::string S;
   generateIRRepresentation(IR, "Initial IR", S);

llvm/lib/Support/VirtualFileSystem.cpp

RedirectingFileSystem::setCurrentWorkingDirectory(const Twine &Path) {
   return {};
 }
 
 std::error_code RedirectingFileSystem::isLocal(const Twine &Path_,
                                                bool &Result) {
   SmallString<256> Path;
   Path_.toVector(Path);
 
-  if (std::error_code EC = makeCanonical(Path))
+  if (makeCanonical(Path))
     return {};
 
   return ExternalFS->isLocal(Path, Result);
 }
 
 std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl<char> &Path) const {
   // is_absolute(..., Style::windows_*) accepts paths with both slash types.
   if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) ||

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

for (VPRecipeBase *CurrentLink : Worklist.getArrayRef().drop_front()) {
         VPInstruction *FMulRecipe = new VPInstruction(
             Instruction::FMul,
             {CurrentLink->getOperand(0), CurrentLink->getOperand(1)},
             CurrentLinkI->getFastMathFlags());
         LinkVPBB->insert(FMulRecipe, CurrentLink->getIterator());
         VecOp = FMulRecipe;
       } else {
         if (RecurrenceDescriptor::isMinMaxRecurrenceKind(Kind)) {
-          if (auto *Cmp = dyn_cast<VPWidenRecipe>(CurrentLink)) {
+          if (isa<VPWidenRecipe>(CurrentLink)) {
             assert(isa<CmpInst>(CurrentLinkI) &&
                    "need to have the compare of the select");
             continue;
           }
           assert(isa<VPWidenSelectRecipe>(CurrentLink) &&
                  "must be a select recipe");
           IndexOfFirstOperand = 1;
         } else {

llvm/tools/llvm-profgen/ProfiledBinary.cpp

if (!DebugContext)
     exitWithError("Error creating the debug info context", Path);
 
   for (const auto &CompilationUnit : DebugContext->compile_units())
     loadSymbolsFromDWARFUnit(*CompilationUnit.get());
 
   // Handles DWO sections that can either be in .o, .dwo or .dwp files.
   for (const auto &CompilationUnit : DebugContext->compile_units()) {
     DWARFUnit *const DwarfUnit = CompilationUnit.get();
-    if (std::optional<uint64_t> DWOId = DwarfUnit->getDWOId()) {
+    if (DwarfUnit->getDWOId()) {
       DWARFUnit *DWOCU = DwarfUnit->getNonSkeletonUnitDIE(false).getDwarfUnit();
       if (!DWOCU->isDWOUnit()) {
         std::string DWOName = dwarf::toString(
             DwarfUnit->getUnitDIE().find(
                 {dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}),
             "");
         WithColor::warning()
             << "DWO debug information for " << DWOName