[clang-tidy] Fix bugprone-use-after-move when move is in noexcept operator

[njames93]

Summary

Fixes noexcept operator misinterpreted as being evaluated.

[• Quuxplusone]

clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

387

What about sizeof, alignof, requires, typeid, and other such unevaluated contexts? Shouldn't there be a common way to spell "this expression is unevaluated"? (I don't know if there is or not.)

[merge_guards_bot]

Unit tests: pass. 62199 tests passed, 0 failed and 815 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[gribozavr2]

clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

387

+1 to adding handling and tests for other unevaluated contexts.

clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

1276

Is the macro a necessary part of this test? If not, can it be removed?

[njames93]

clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

387

I think i can write a matcher that will detect that

clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

1276

I like to stick to the bug report as much as possible

[gribozavr2]

clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

1276

This is not generally the policy in LLVM and Clang. We prefer minimal reproducers.

[njames93]

• added more unevaluated context checks

[njames93]

clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

1276

I've kept it minimal, the REQUIRE definition is just to suppress other warnings about unused expressions

[njames93]

• Elide braces

[merge_guards_bot]

Unit tests: pass. 62207 tests passed, 0 failed and 815 were skipped.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[merge_guards_bot]

Unit tests: pass. 62206 tests passed, 0 failed and 815 were skipped.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[njames93]

• Fix formatting

[merge_guards_bot]

Unit tests: pass. 62207 tests passed, 0 failed and 815 were skipped.

clang-tidy: pass.

clang-format: fail. Please format your changes with clang-format by running git-clang-format HEAD^ or applying this patch.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[gribozavr2]

LGTM with fixes to the test.

clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

1276

Adding (void) should do the trick.

1279

I'm not sure why foo and otherUnenvaluated are separate functions. If you want to separate them, I think it would make more sense to put each operator into a separate function then.

[njames93]

• Fix nits in test cases

[merge_guards_bot]

Unit tests: pass. 62213 tests passed, 0 failed and 815 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

[njames93]

• Remove artifacts of old dependent review

[merge_guards_bot]

Unit tests: pass. 62249 tests passed, 0 failed and 816 were skipped.

clang-tidy: pass.

clang-format: pass.

Build artifacts: diff.json, clang-tidy.txt, clang-format.patch, CMakeCache.txt, console-log.txt, test-results.xml

Pre-merge checks is in beta. Report issue. Please join beta or enable it for your project.

clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp

 //===--- UseAfterMoveCheck.cpp - clang-tidy -------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "UseAfterMoveCheck.h"
 
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Analysis/CFG.h"
 #include "clang/Lex/Lexer.h"
 
 #include "../utils/ExprSequence.h"
 
 using namespace clang::ast_matchers;
 using namespace clang::tidy::utils;
 
void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
   if (!getLangOpts().CPlusPlus11)
     return;
 
   auto CallMoveMatcher =
       callExpr(callee(functionDecl(hasName("::std::move"))), argumentCountIs(1),
                hasArgument(0, declRefExpr().bind("arg")),
                anyOf(hasAncestor(lambdaExpr().bind("containing-lambda")),
                      hasAncestor(functionDecl().bind("containing-func"))),
-               unless(inDecltypeOrTemplateArg()))
+               unless(inDecltypeOrTemplateArg()),
+               unless(hasAncestor(cxxNoexceptExpr())))
           .bind("call-move");
 
   Finder->addMatcher(
       // To find the Stmt that we assume performs the actual move, we look for
       // the direct ancestor of the std::move() that isn't one of the node
       // types ignored by ignoringParenImpCasts().
       stmt(forEach(expr(ignoringParenImpCasts(CallMoveMatcher))),
            // Don't allow an InitListExpr to be the moving call. An InitListExpr

clang-tools-extra/test/clang-tidy/checkers/bugprone-use-after-move.cpp

 class C : T, B {
   void m_fn1() {
     int a;
     std::move(a);
     C c;
   }
 };
 }
+
+namespace PR44667 {
+#define REQUIRE(expr) (void)(expr);
+struct S {};
+
+void foo() {
+  S s;
+  REQUIRE(noexcept(S{std::move(s)}));
+  S other{std::move(s)};
+}
+} // namespace PR44667