[PATCH] D75250: [DebugInfo] Add unit test for compact expression printer

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp:27
+public:
+  std::unique_ptr<MCRegisterInfo> MRI;
+
----------------
As this is a brand new file, and is completely local in scope (i.e. doesn't get used by anything else), perhaps we should apply the planned new variable naming rules of lower camel-case for variable names (e.g. `mri`, `tripleName` etc)? What do you think?

I'm okay with it either way.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFExpressionCompactPrinterTest.cpp:41-42
+    // If we didn't build ARM, do not run the test.
+    if (!TheTarget)
+      return;
+
----------------
Are you sure this won't run the test here? I think this will just stop the constructor early, but the test will still be run.

I think you need this return inside the `TEST_F` blocks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75250/new/

https://reviews.llvm.org/D75250