This is an archive of the discontinued LLVM Phabricator instance.

[lldb] Make expect_expr fall back to the dummy target if no target is selected
ClosedPublic

Authored by teemperor on Jul 8 2020, 5:11 AM.

Details

Summary

Currently expect_expr will not run the expression if no target is selected. This patch changes
this behavior so that expect_expr will instead fall back to the dummy target similar to what
the expression command is doing. This way we don't have to compile an empty executable
to be able to use expect_expr (which is a waste of resources for tests that just test
generic type system features).

As a test I modernized the TestTypeOfDeclTypeExpr into a Python test + expect_expr
(as it relied on the dummy target fallback of the expression command).

Diff Detail

Event Timeline

teemperor created this revision.Jul 8 2020, 5:11 AM
labath accepted this revision.Jul 8 2020, 8:09 AM

The expect_expr functionality is fine. Though I don't have a particular problem with the rewrite of this test (it does fit in nicely into the existing lang/cpp/ hierarchy, I think it would be good to clarify the guidelines for what tool to use for tests like these. Otherwise, I think we'll just go on rewriting tests in circles. Looking at the testing webpage, this seems to be a pretty clear cut case of the "A good rule of thumb is to prefer shell tests when what is being tested is relatively simple." rule. I think most people are obeying that rule (even if they don't know that it exists, and some go even too far IMO), but you're not one of those people...

The reason I prefer shell tests is that api tests make it very easy to tie your test to the specifics of one platform (os, architecture), and then slap @skipUnlessMyPlatform on the test. That means that anyone not running that platform will not be able to run that test (and see if it broke it). With shell tests, and some care, a lot of these tests could be written to not depend on a particular host platform, usually by hardcoding a target triple in the compiler invocation. This means that one can run arm64 darwin tests on an x86 windows host. Obviously there's value in the "api" version of these tests too, but I'm pushing for the shell versions because we have so few of those. So it's e.g. impossible to check whether something breaks ObjectFileMachO functionality without a darwin host, even though most of that code is perfectly testable without it.

None of that applies to this test though...

This revision is now accepted and ready to land.Jul 8 2020, 8:09 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 15 2020, 4:56 AM
lldb/packages/Python/lldbsuite/test/lldbtest.py