-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added support for casting with auto declaration #710
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you change the code according to my suggestions, also add tests
{ | ||
auto t2 = castExpression.type.type2; | ||
|
||
auto l = symbol.typeLookups.front; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
according to other code in this file typeLookups could be empty. I'm not sure when this would be the case, if you know you could you add a test? Also I would recommend adding a check that aborts or inserts an empty typeLookup
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's never empty, it always gets inserted here:
DCD/dsymbol/src/dsymbol/conversion/first.d
Lines 259 to 267 in 1c54fc9
if (dec.autoDeclaration !is null) | |
{ | |
foreach (part; dec.autoDeclaration.parts) | |
{ | |
SemanticSymbol* symbol = allocateSemanticSymbol( | |
part.identifier.text, CompletionKind.variableName, | |
symbolFile, part.identifier.index); | |
symbol.parent = currentSymbol; | |
populateInitializer(symbol, part.initializer); |
DCD/dsymbol/src/dsymbol/conversion/first.d
Lines 1085 to 1092 in 1c54fc9
void populateInitializer(T)(SemanticSymbol* symbol, const T initializer, | |
bool appendForeach = false) | |
{ | |
auto lookup = TypeLookupsAllocator.instance.make!TypeLookup(TypeLookupKind.initializer); | |
scope visitor = new InitializerVisitor(lookup, appendForeach, this); | |
symbol.typeLookups.insert(lookup); | |
visitor.visit(initializer); | |
} |
l.breadcrumbs.clear(); | ||
void buildChain(TypeLookup* lookup, TypeIdentifierPart tip) | ||
{ | ||
if (tip.identifierOrTemplateInstance) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a TODO for supporting tip.indexer
(D syntax is IdentifierOrTemplateInstance[indexer].typeIdentifierPart
)
{ | ||
if (tip.identifierOrTemplateInstance) | ||
{ | ||
auto crumb = tip.identifierOrTemplateInstance.identifier.text; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
identifier might not be set (in that case .templateInstance is set)
syntax:
identifier.identifier.templateInstance!(args).identifier
} | ||
|
||
if (t2 && t2.typeIdentifierPart) | ||
buildChain(l, t2.typeIdentifierPart); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
t2
has a member called dot
, which means the type starts with a leading dot, e.g. .MyType
This syntax is used to indicate global namespacing, so I think we should add something to the breadcrumbs here to support accessing the global namespace (or if it is global right now, add support to access the local one when it's not dot)
or add a TODO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh that's a good point, i'll try to add support for that, or if i can't i'll leave a //TODO note
rework of dlang-community#710, reuses the existing type construction Co-authored-by: ryuukk <ryuukk.dev@gmail.com>
rework of #710, reuses the existing type construction Co-authored-by: ryuukk <ryuukk.dev@gmail.com>
merged in #766 |
That was easy
One step closer to full template support