This is an archive of the discontinued LLVM Phabricator instance.

Refactor GetNextPersistentVariableName into a non-virtual method
ClosedPublic

Authored by aprantl on Apr 25 2018, 3:17 PM.

Details

Summary

Refactor GetNextPersistentVariableName into a non-virtual method
that takes a prefix string. This simplifies the implementation and
allows plugins such as the Swift plugin to supply different prefixes
for return and error variables.

rdar://problem/39722386

Diff Detail

Repository
rL LLVM

Event Timeline

jingham requested changes to this revision.Apr 27 2018, 5:17 PM

I'm curious how you are going to implement GetPersistentVariablePrefix for a language (e.g. swift) that uses multiple prefixes. You've got a pattern here where you always call GetPersistentVariablePrefix, but you're going to have to break that pattern for swift. And then how are you going to handle that in generic code w/o having to check the language.

Anyway, you shouldn't hardcode $ twice.

source/Plugins/ExpressionParser/Clang/ClangUserExpression.cpp
672 ↗(On Diff #144022)

Was it just an oversight that you don't call m_persistent_state->GetPersistentVariablePrefix here?

This revision now requires changes to proceed.Apr 27 2018, 5:17 PM
aprantl updated this revision to Diff 144649.Apr 30 2018, 4:20 PM

Address review feedback.

jingham accepted this revision.Apr 30 2018, 4:34 PM

That's fine.

This revision is now accepted and ready to land.Apr 30 2018, 4:34 PM
This revision was automatically updated to reflect the committed changes.