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.

Sign up for GitHub

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

Jump to bottom

Extract include func to dsymbolsem #16970

Draft
wants to merge 10 commits into
base: master
Choose a base branch
from
+140 −99
Draft

Extract include func to dsymbolsem #16970

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
bab81b4
Extract include func to dsymbolsem
dchidindu5 Oct 7, 2024
beccdbd
Extract include func to dsymbolsem
dchidindu5 Oct 7, 2024
88facde
Extract include func to dsymbolsem
dchidindu5 Oct 8, 2024
acaf751
Extract include func to dsymbolsem
dchidindu5 Oct 10, 2024
53035de
Extract include func to dsymbolsem
dchidindu5 Oct 11, 2024
20c81ba
Extract include func to dsymbolsem
dchidindu5 Oct 11, 2024
b1e3b2f
Fix statement is not reachable error
dchidindu5 Oct 13, 2024
9bbf4df
Add return to conddecl
dchidindu5 Oct 14, 2024
4818c16
Extract include func to dsymbolsem
dchidindu5 Oct 16, 2024
647bc27
Extract include func to dsymbolsem
dchidindu5 Oct 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 3 additions & 77 deletions compiler/src/dmd/attrib.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import dmd.declaration;
import dmd.dmodule;
import dmd.dscope;
import dmd.dsymbol;
import dmd.dsymbolsem : setScope, addMember;
//import dmd.dsymbolsem : setScope, addMember;
import dmd.expression;
import dmd.func;
import dmd.globals;
Expand Down Expand Up @@ -701,8 +701,8 @@ extern (C++) class ConditionalDeclaration : AttribDeclaration
extern (C++) final class StaticIfDeclaration : ConditionalDeclaration
{
ScopeDsymbol scopesym; /// enclosing symbol (e.g. module) where symbols will be inserted
private bool addisdone = false; /// true if members have been added to scope
private bool onStack = false; /// true if a call to `include` is currently active
public bool addisdone = false; /// true if members have been added to scope
public bool onStack = false; /// true if a call to `include` is currently active
dchidindu5 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also be sure to update attrib.h and frontend.h to reflect the changes made

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd do that once I'm done with the refactoring


extern (D) this(const ref Loc loc, Condition condition, Dsymbols* decl, Dsymbols* elsedecl) @safe
{
Expand All @@ -716,42 +716,6 @@ extern (C++) final class StaticIfDeclaration : ConditionalDeclaration
return new StaticIfDeclaration(loc, condition.syntaxCopy(), Dsymbol.arraySyntaxCopy(decl), Dsymbol.arraySyntaxCopy(elsedecl));
}

/****************************************
* Different from other AttribDeclaration subclasses, include() call requires
* the completion of addMember and setScope phases.
*/
override Dsymbols* include(Scope* sc)
{
//printf("StaticIfDeclaration::include(sc = %p) scope = %p\n", sc, _scope);

if (errors || onStack)
return null;
onStack = true;
scope(exit) onStack = false;

if (sc && condition.inc == Include.notComputed)
{
assert(scopesym); // addMember is already done
assert(_scope); // setScope is already done
Dsymbols* d = ConditionalDeclaration.include(_scope);
if (d && !addisdone)
{
// Add members lazily.
d.foreachDsymbol( s => s.addMember(_scope, scopesym) );

// Set the member scopes lazily.
d.foreachDsymbol( s => s.setScope(_scope) );

addisdone = true;
}
return d;
}
else
{
return ConditionalDeclaration.include(sc);
}
}

override const(char)* kind() const
{
return "static if";
Expand Down Expand Up @@ -818,43 +782,6 @@ extern (C++) final class StaticForeachDeclaration : AttribDeclaration
return false;
}

override Dsymbols* include(Scope* sc)
{
if (errors || onStack)
return null;
if (cached)
{
assert(!onStack);
return cache;
}
onStack = true;
scope(exit) onStack = false;

if (_scope)
{
sfe.prepare(_scope); // lower static foreach aggregate
}
if (!sfe.ready())
{
return null; // TODO: ok?
}

// expand static foreach
import dmd.statementsem: makeTupleForeach;
Dsymbols* d = makeTupleForeach(_scope, true, true, sfe.aggrfe, decl, sfe.needExpansion).decl;
if (d) // process generated declarations
{
// Add members lazily.
d.foreachDsymbol( s => s.addMember(_scope, scopesym) );

// Set the member scopes lazily.
d.foreachDsymbol( s => s.setScope(_scope) );
}
cached = true;
cache = d;
return d;
}

override void addComment(const(char)* comment)
{
// do nothing
Expand Down Expand Up @@ -921,7 +848,6 @@ extern(C++) final class ForwardingAttribDeclaration : AttribDeclaration
}
}


/***********************************************************
* Mixin declarations, like:
* mixin("int x");
Expand Down
89 changes: 89 additions & 0 deletions compiler/src/dmd/dsymbolsem.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7475,6 +7475,95 @@ private extern(C++) class NewScopeVisitor : Visitor
sc = sc2;
}
}

extern(C++) Dsymbols* include(Dsymbol d, Scope* sc)
{
scope icv = new IncludeVisitor(sc);
d.accept(icv);
return icv.sc;
}

extern(C++) class IncludeVisitor : Visitor
{
alias visit = typeof(super).visit;
Scope* sc;
this(Scope* sc)
{
this.sc = sc;
}

override void visit(StaticIfDeclaration sfd)
thewilsonator marked this conversation as resolved.
Show resolved Hide resolved
{
/****************************************
* Different from other AttribDeclaration subclasses, include() call requires
* the completion of addMember and setScope phases.
*/
//printf("StaticIfDeclaration::include(sc = %p) scope = %p\n", sc, _scope);
if (sfd.errors || sfd.onStack)
//return null;
sfd.onStack = true;
scope(exit) sfd.onStack = false;

if (sc && condition.inc == Include.notComputed)
{
assert(sfd.scopesym); // addMember is already done
assert(_scope); // setScope is already done
thewilsonator marked this conversation as resolved.
Show resolved Hide resolved
d = ConditionalDeclaration.include(_scope);
thewilsonator marked this conversation as resolved.
Show resolved Hide resolved
if (d && !addisdone)
{
// Add members lazily.
sfd.d.foreachDsymbol( s => s.addMember(_scope, scopesym) );

// Set the member scopes lazily.
d.foreachDsymbol( s => s.setScope(_scope) );

addisdone = true;
}
return d;
}
else
{
return ConditionalDeclaration.include(sc);
thewilsonator marked this conversation as resolved.
Show resolved Hide resolved
}
}

override void visit(StaticForeachDeclaration sed)
thewilsonator marked this conversation as resolved.
Show resolved Hide resolved
{
if (errors || onStack)
thewilsonator marked this conversation as resolved.
Show resolved Hide resolved
return null;
if (cached)
{
assert(!onStack);
return cache;
}
Copy link
Contributor

@RazvanN7 RazvanN7 Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is equivalent to:

if (sfd.errors || sfd.onStack)
{
        if (sfd.cached)
        {
            assert(!sfd.onStack);
            symbols= sfd.cache;
        }
}

Whereas what you want is:

if (sfd.errors || sfd.onStack)
{
      symbols = null;
      return;
}
if (sfd.cached)
{
       assert(!sfd.onStack);
       symbols= sfd.cache;
       return;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also symbols = sfd.cache; with proper spaces.

onStack = true;
scope(exit) onStack = false;

if (_scope)
{
sfe.prepare(_scope); // lower static foreach aggregate
}
if (!sfe.ready())
{
return null; // TODO: ok?
}

// expand static foreach
import dmd.statementsem: makeTupleForeach;
Dsymbols* d = makeTupleForeach(_scope, true, true, sfe.aggrfe, decl, sfe.needExpansion).decl;
if (d) // process generated declarations
{
// Add members lazily.
sed.d.foreachDsymbol( s => s.addMember(_scope, scopesym) );

// Set the member scopes lazily.
d.foreachDsymbol( s => s.setScope(_scope) );
}
cached = true;
cache = d;
return d;
}
}
/**
* Called from a symbol's semantic to check if `gnuAbiTag` UDA
* can be applied to them
Expand Down
Loading