Add ImportLocation to SymbolToken #12
Conversation
avneet-toor-bq
force-pushed
the
token-latest-fix
branch
2 times, most recently
from
4752d00
to
e52f9d8
Compare
There was a problem hiding this comment.
This is looking good. Here are two bits of logic I don't see in here:
- When the reader encounters a symbol ID that occurs within a shared symbol table but that shared symbol table is not resolved in the catalog, the reader should return a SymbolToken with
nulltext and an ImportLocation that identifies the name and SID in the unresolved shared symbol table. - When the writer is provided with a SymbolToken with
nulltext and with a valid ImportLocation, it should calculate the local symbol ID from the import symbol ID and write the local symbol ID.
This can be done in a separate CR if your prefer.
Sure. |
I've created #31 for this. |
| @@ -704,8 +704,8 @@ public IEnumerable<SymbolToken> GetTypeAnnotations() | |||
| throw new UnknownSymbolException(a.Sid); | |||
| } | |||
|
|
|||
| var text = symtab.FindKnownSymbol(a.Sid); | |||
| yield return new SymbolToken(text, a.Sid); | |||
| var text = symtab.FindKnownSymbol(a.ImportLocation.Sid); | |||
There was a problem hiding this comment.
One last thing, and then this is good to go:
Line 699 should be if (a.Text is null && a.ImportLocation != null) since we're relying on ImportLocation now and not local SID.
There was a problem hiding this comment.
Did you mean if (a.Text is null && a.ImportLocation != default)? Or is null what we are checking for in this instance?
There was a problem hiding this comment.
We want to know if a.ImportLocation is specified. If a.ImportLocation != default is the correct way to do that, then yes that's what I mean.
No description provided.