Page MenuHomePhabricator

Refactor GetNextPersistentVariableName into a non-virtual method

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



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.


Diff Detail


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.

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.