Page MenuHomePhabricator

[lldb] Add a C language REPL to test LLDB's REPL infrastructure
Needs ReviewPublic

Authored by teemperor on Sep 8 2020, 5:26 AM.

Details

Summary

LLDB has a bunch of code that implements REPL support, but all that code is unreachable as no language
in master currently has an implemented REPL backend. The only REPL that exists is in the downstream
Swift fork. All patches for this generic REPL code therefore also only have tests downstream which is
clearly not a good situation.

This patch implements a basic C language REPL on top of LLDB's REPL framework. Beside implementing
the REPL interface and hooking it up into the plugin manager, the only other small part of this patch
is making the --language flag of the expression command compatible with the --repl flag. The --repl
flag uses the value of --language to see which REPL should be started, but right now the --language
flag is only available in OptionGroups 1 and 2, but not in OptionGroup 3 where the --repl flag is
declared.

The REPL currently can currently only start if a running target exists. I'll add the 'create and run a dummy executable'
logic from Swift (which is requires when doing lldb --repl) when I have time to translate all this logic
to something that will work with Clang.

I should point out that the REPL currently uses the C expression parser's approach to persistent variables
where only result variables and the ones starting with a '$' are transferred between expressions. I'll fix
that in a follow up patch. Also the REPL currently doesn't work in a non-interactive terminal. This seems to
be fixed in the Swift fork, so I assume one of our many REPL downstream changes addresses the issue.

Diff Detail

Event Timeline

teemperor created this revision.Sep 8 2020, 5:26 AM
teemperor requested review of this revision.Sep 8 2020, 5:26 AM

(And I'm aware there are a lot more tests needed to test all the REPL code that had no tests so far. I'll see what's missing once this his the coverage bot and then I'll make put reviews up for that).

mib added a comment.Sep 8 2020, 6:53 AM

Some nitpicking but this looks fine to me.

lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
79

I guess this is not the final implementation, so may be add a // FIXME

mib added inline comments.Sep 8 2020, 6:54 AM
lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
65
$ swift
Welcome to Apple Swift version 5.3 (swiftlang-1200.0.29.2 clang-1200.0.30.1).
Type :help for assistance.
  1> "\(#file)" 
$R0: String = "repl.swift"

To keep consistency with the SwiftREPL, maybe the REPL source file should be named repl.c

teemperor updated this revision to Diff 290478.Sep 8 2020, 7:09 AM
  • Added comments to unimplemented functions.
  • Changed repl source file name.
teemperor updated this revision to Diff 290479.Sep 8 2020, 7:12 AM
  • Retry uploading diff.
teemperor set the repository for this revision to rLLDB LLDB.Sep 8 2020, 7:29 AM
mib added a reviewer: jingham.Sep 8 2020, 7:32 AM
mib added a project: Restricted Project.

This looks mostly good! I just have a few minor nitpicks.

lldb/source/Plugins/REPL/Clang/ClangREPL.cpp
28

We recently added a isFortran() function to Dwarf.h in llvm... We might want to add a isCplusplus() function, too?
Hmm.. but looping over all constants and asking isCpluPlus() seems dumb, too. Otoh — I'm convinced that we'll forget to add C++17 here in the future....

lldb/source/Plugins/REPL/Clang/ClangREPL.h
15

Doxygen comment(s)?

30

This is obviously not important at all — I found some plugins that seem to just return a char * or a StringRef here. If that is possible I guess that would be micro-optimization we could do here, too?

lldb/test/API/repl/clang/TestClangREPL.py
4

Wheee! PExpect ...