Page MenuHomePhabricator

Move ExpressionSourceCode.cpp -> ClangExpressionSourceCode.cpp
ClosedPublic

Authored by jingham on Mar 6 2019, 12:19 PM.

Details

Summary

ExpressionSourceCode.cpp is in source/Expressions - which should indicate that it is a generic part of the expression parser. In fact, almost everything it currently does is very Clang specific. This change factors out the clang specific bits into ClangExpressionSourceCode.cpp. It is an NFC change, but it will help to reduce conflicts in supporting other Expression Parsers (e.g. swift).

I am leaving ExpressionSourceCode.h in place even though at present it doesn't have much content. In the swift.org repository we have support for emitting debug information for compiled expressions - which requires saving out the expression text, and that is handled by this class. That's a sometimes handy feature. This change will make it easier to merge in that facility, though I'll do that as a separate commit.

Diff Detail

Repository
rLLDB LLDB

Event Timeline

jingham created this revision.Mar 6 2019, 12:19 PM

I forgot to sort the project file before making the diff. I'll do that on commit.

aprantl added inline comments.Mar 6 2019, 12:53 PM
source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
137

This else return doesn't do anything. Is it needed for swift-lldb? Otherwise I'd just delete it.

source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
2

Level 10 nitpick: can you delete a few - characters so this fits into 80 columns?

43

///

source/Plugins/ExpressionParser/Clang/ClangUtilityFunction.cpp
45

Optional: If we changed the interface to llvm::StringRef() we don't have to check for a nullptr.

shafik accepted this revision.Mar 6 2019, 1:01 PM

LGTM although there are some good comments by @aprantl

This revision is now accepted and ready to land.Mar 6 2019, 1:01 PM
jingham marked 3 inline comments as done.Mar 6 2019, 2:39 PM

I want to keep this commit simple so I'm leaving Adrian's substantial comments for a future commit.

source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.cpp
137

This is not required for swift, this is all C code and is just a straight copy of what used to be in ExpressionSourceCode.cpp. It's just the way Sean wrote it. I would rather not add extra differences before merging this back to swift, but I've made a note to fix it once I get that done.

source/Plugins/ExpressionParser/Clang/ClangExpressionSourceCode.h
2

I'll do that on checkin.

This revision was automatically updated to reflect the committed changes.