Skip to content

Comments

[cleanup] Finish moving glue functions out of classes#4291

Merged
9rnsr merged 16 commits intodlang:masterfrom
yebblies:moreglue
Jan 15, 2015
Merged

[cleanup] Finish moving glue functions out of classes#4291
9rnsr merged 16 commits intodlang:masterfrom
yebblies:moreglue

Conversation

@yebblies
Copy link
Contributor

Net -216 lines.

@yebblies yebblies force-pushed the moreglue branch 2 times, most recently from 6d59497 to b284b37 Compare January 14, 2015 12:33
@yebblies
Copy link
Contributor Author

Green now, I missed renaming one of the local variables that conflicted.

With this, gluestub is looking very light!

We have some functions to trigger typeinfo/helper/init/etc generation:

toInitializer
toModuleAssert
toModuleUnittest
toModuleArray
getInternalTypeInfo
getTypeInfo

Backend/object emission:

backend_init
backend_term
obj_start
obj_end
obj_write_deferred
genObjFile
genhelpers

Library stuff (this should probably be in frontend/driver code)

LibMSCoff_factory
LibOMF_factory
LibElf_factory
LibMach_factory

Inline asm: asmSemantic (should probably be in the frontend)
And one function that should probably be in Target: retStyle

@dnadlinger
Copy link
Contributor

Looks good from the LDC point of view. We have our own (currently #ifdef'd) analogues for the *Symbol methods anyway.

@yebblies
Copy link
Contributor Author

Cool. Is there an easy way to see a diff with LDC's current frontend patches?

@ibuclaw
Copy link
Member

ibuclaw commented Jan 14, 2015

I'll need to update my PR diff. However it will not be representative of the current situation - just what the difference should be after I get round to the many, many visitor conversions.

@yebblies - Apart from what you can see when you search for IN_GCC in dmd, there is ExtAsmStatement class, and LDC has it's own AsmBlockStatement too. IIRC, I seem to recall the general consensus being to add these to statement.h so they get included in the ddmd shared source (but version'd out).

@yebblies
Copy link
Contributor Author

I was thinking we add parser support for ExtAsmStatement/AsmBlockStatement, if they don't parse already, then stub them out in semantic for dmd.

Visitors have solved the problem of adding methods fairly well, but we still have the problem of storing backend-dependent state. I'm thinking of Type::ctype, Dsymbol::csym and similar. What do you think about just moving them into hash tables?

@ibuclaw
Copy link
Member

ibuclaw commented Jan 14, 2015

It would have to be a void* if it goes in shared code. Otherwise, we could have a class Backend that I mentioned in another PR. ie:

class Dsymbol : public xxx
{
...
  Backend btype;
}

And in dmd-specific headers:

class DMDBackend : public Backend
{
...
}

Would that go down well?

@yebblies
Copy link
Contributor Author

My concern is that that doesn't scale well. We'd need a different type for every ast class that needs to store data, and while it makes sense to group all the variable the backend needs together, I'm hoping there will be (many) other consumers in the future.

@ibuclaw
Copy link
Member

ibuclaw commented Jan 14, 2015

@yebblies - I only suggest as GDC uses the same type across the board for all kinds of symbols - decls, types, etc... so it makes most logical sense for us. :)
How can you achieve it in dmd? You could use a union maybe...

@andralex
Copy link
Member

noice

@yebblies
Copy link
Contributor Author

Sooo anyone want to merge this or should I start splitting it up?

@9rnsr
Copy link
Contributor

9rnsr commented Jan 15, 2015

Looks good to me.

@9rnsr
Copy link
Contributor

9rnsr commented Jan 15, 2015

Auto-merge toggled on

@9rnsr
Copy link
Contributor

9rnsr commented Jan 15, 2015

Auto-merge toggled off

@9rnsr
Copy link
Contributor

9rnsr commented Jan 15, 2015

Auto-merge toggled on

@yebblies
Copy link
Contributor Author

Thanks!

9rnsr added a commit that referenced this pull request Jan 15, 2015
[cleanup] Finish moving glue functions out of classes
@9rnsr 9rnsr merged commit 3dcdb16 into dlang:master Jan 15, 2015
@yebblies yebblies deleted the moreglue branch January 15, 2015 10:30
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.

5 participants