Skip to content
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

Closed
wants to merge 5 commits into from
Closed

Added support for casting with auto declaration #710

wants to merge 5 commits into from

Conversation

ryuukk
Copy link
Contributor

@ryuukk ryuukk commented Feb 9, 2023

That was easy

One step closer to full template support

Code_QH9i0Jp3Hn

Copy link
Member

@WebFreak001 WebFreak001 left a 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;
Copy link
Member

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

Copy link
Contributor Author

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:

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);

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)
Copy link
Member

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;
Copy link
Member

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);
Copy link
Member

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

Copy link
Contributor Author

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

@ryuukk ryuukk closed this by deleting the head repository Feb 14, 2023
@ryuukk ryuukk reopened this Feb 14, 2023
WebFreak001 added a commit to WebFreak001/DCD that referenced this pull request Dec 4, 2023
rework of dlang-community#710, reuses the existing type construction

Co-authored-by: ryuukk <ryuukk.dev@gmail.com>
WebFreak001 added a commit that referenced this pull request Dec 4, 2023
rework of #710, reuses the existing type construction

Co-authored-by: ryuukk <ryuukk.dev@gmail.com>
@WebFreak001
Copy link
Member

merged in #766

@WebFreak001 WebFreak001 closed this Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants