This is an archive of the discontinued LLVM Phabricator instance.

Fix broken builtin functions in the expression command
ClosedPublic

Authored by teemperor on Aug 8 2018, 4:47 PM.

Details

Summary

Calling any non-libc builtin function in the expression command currently just causes Clang
to state that the function is not known. The reason for this is that we actually never
initialize the list of builtin functions in the Builtin::Context.

This patch just calls the initializer for the builtins in the preprocessor. Also adds some tests
for the new builtins.

It also gets rid of the extra list of builtins in the ClangExpressionParser, as we can just reuse
the existing list in the Preprocessor for the ASTContext. Having just one list of builtins around
is also closer to the standard Clang behavior.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

teemperor created this revision.Aug 8 2018, 4:47 PM

I didn't disable Windows on the test as it shouldn't call any function in the target process. Just because I don't want to x-fail every test on Windows.

teemperor edited reviewers, added: Restricted Project; removed: jingham.Aug 22 2018, 1:28 PM

(Adding the team as a reviewer, because the code this patch touches doesn't seem to have a dedicated code owner).

vsk accepted this revision as: vsk.Aug 22 2018, 1:39 PM
vsk added a subscriber: vsk.

Thanks, LGTM!

This revision is now accepted and ready to land.Aug 22 2018, 1:39 PM

Changed look fine. Wondering if we want to be adding pexpect style tests though. Those tests tend to be flaky. This could easily be done with SB API calls instead of using pexpect.

teemperor planned changes to this revision.Aug 22 2018, 2:32 PM

@clayborg Thanks, getting rid of pexpect sounds good.

TODO:

  • Get rid of pexpect.
  • Fix some of the comments in the test that are still referring to the test I copied this from.
  • Reuse builtin_context instead of calling PP.getBuiltinInfo() twice in ClangExpressionParser.cpp.
teemperor updated this revision to Diff 162255.Aug 23 2018, 1:34 PM
  • Removed unrelated comments in the test.
  • No longer using self.expect.
  • Small refactoring in the parsing code.
This revision is now accepted and ready to land.Aug 23 2018, 1:34 PM
This revision was automatically updated to reflect the committed changes.
teemperor added a project: Restricted Project.Aug 30 2018, 12:44 PM