NoQ added a comment.
Hey, thanks! That's a nice catch.
The code looks fine, but i don't think your test actually tests it - see inline
comments. Also i think we should put the test into the clang repo (i.e.,
`test/Analysis`), because that's where the change is. I'd like to know it if i
break your test even if i don't enable clang-tools-extra at all. Because
clang-tidy isn't available from within the clang sub-project, this would have
to be implemented as a static analyzer test.
================
Comment at: clang-tools-extra/test/clang-tidy/asm-goto.cpp:2
+// REQUIRES: static-analyzer
+// RUN: clang-tidy %s -checks='bugprone-use-after-move' -- | FileCheck %s
+#include <string>
----------------
I suspect that the code you modified doesn't get covered by this test unless
you enable `analyzer-...` checkers.
================
Comment at: clang-tools-extra/test/clang-tidy/asm-goto.cpp:3-4
+// RUN: clang-tidy %s -checks='bugprone-use-after-move' -- | FileCheck %s
+#include <string>
+#include <utility>
+int main() {
----------------
This is scary because the behavior may depend on the implementation details of
the standard library that's installed on the current machine. In the Static
Analyzer we use `test/Analysis/Inputs/system-header-simulator*.h` as a
mocked-up standard library for testing purposes.
================
Comment at: clang/lib/StaticAnalyzer/Core/CoreEngine.cpp:401
+ case Stmt::GCCAsmStmtClass:
+ return;
}
----------------
Please add a TODO to actually implement this functionality.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D63533/new/
https://reviews.llvm.org/D63533
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
