[PATCH] D63533: [analyzer] Fix clang-tidy crash on GCCAsmStmt

[Artem Dergachev via Phabricator via cfe-commits]

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