Page MenuHomePhabricator

Use symbols with the default version to resolve unversioned symbols.
Needs ReviewPublic

Authored by ruiu on Jun 9 2017, 3:07 PM.



This patch is made based on

Symbols can contain symbol versions in themselves. If a symbol name
is in the form of "foo@bar", foo's version is "bar".

There's one special version -- the default version. If a symbol is in
the form of "foo@@bar", it is used not only to resolve "foo" in
version "bar" but also any unversioned "foo". Thus, it is called the

This patch implements the behavior.


Event Timeline

ruiu created this revision.Jun 9 2017, 3:07 PM
silvas added a subscriber: silvas.Jun 9 2017, 7:18 PM

If it is based on D33680 I presume it fixes also; would be good to mention that in the description too.

grimar edited edge metadata.Jun 9 2017, 11:31 PM

Thanks for taking a look on that ! Really.

Honestly I think D33680 is more clear for my eye. Let me do one more update for it,
I have one change in mind I did not include ealier, we can discuss which way fits better after that.

My conserns about this patch are inline.


I would inline it.


So you are mutating arguments here. Generally you are asking to avoid that in reviews.
And I think I agree with that strategy.
Mutating arguments really made me be confusing, please see next comment.


So Version can have valid 0 value it seems ?

But below you do check for 0:

if (Ver != 0)
  Sym->VersionId = IsDefault ? Ver : (Ver | VERSYM_HIDDEN);

That become visible and confusing for me mostly because of mutating arguments.
(I was a bit lost in what this method do in general).

ruiu added inline comments.Jun 12 2017, 10:18 AM

I wouldn't -- I tried both and it seems this is slightly clearner.


Well, I tried to use a tuple or a struct to return a value, but turned out this seems the best option I have.


I could fix it.

grimar added inline comments.Jun 12 2017, 10:39 AM

I have one idea in mind for D33680, I would like to show you, sorry again for delay here. I'll polish and update it tomorrow to demonstrate and we can choose which approach fits better then..

ruiu added inline comments.Jun 12 2017, 10:41 AM

Take your time. No reason to rush.