Page MenuHomePhabricator

Stop calling ValueObject::SetName from synthetic child providers

Authored by tberghammer on Mar 25 2017, 2:47 PM.



Calling ValueObject::SetName from a sythetic child provider would change
the underying value object used for the non-synthetic child as well what
is clearly unintentional.

Diff Detail


Event Timeline

tberghammer created this revision.Mar 25 2017, 2:47 PM
jingham requested changes to this revision.Mar 28 2017, 11:56 AM

If SetName is the wrong thing to use for synthetic child providers, then we need to make Clone available to the SB API's as well. In any case where they want to return an extant ValueObject with a different name - as opposed to making one up out of whole cloth - won't they need the same operation?

Also, probably should document this requirement in the varformats.html.

607–610 ↗(On Diff #93058)

Spelling... "he" -> "the", "chane" -> "change", "actul" -> "actual"

This revision now requires changes to proceed.Mar 28 2017, 11:56 AM

SBValue::SetName is not part of the SB API (what is the right decision IMO as an SBValue should be mostly immutable) so this issue doesn't effect it. I looked through the code in examples/synthetic/ and it is always using one of the SBValue::Create* method to produce new SBValue what will create a new value object one way or the other. Considering that nobody complained about the missing SetName method at the SB API level I don't see a big need for exposing the Clone method there. At the same line if SetName/Clone isn't part of the SB API then I think we shouldn't document it at the webpage.

(I will upload a fix for the spelling errors later)

tberghammer edited edge metadata.

Fix typos in the comments

tberghammer marked an inline comment as done.Mar 30 2017, 1:00 PM
jingham accepted this revision.Mar 30 2017, 1:04 PM


This revision is now accepted and ready to land.Mar 30 2017, 1:04 PM
This revision was automatically updated to reflect the committed changes.