[Tinycc-devel] Re: patches for libtcc

[Rob Landley]

On Tuesday 11 September 2007 3:28:54 am Gabriel Corneanu wrote:
> Hi,
>
> Comments are inserted:
>
> On 9/11/07, Rob Landley <address@hidden> wrote:
> > On Monday 10 September 2007 9:14:10 am Gabriel Corneanu wrote:
> > > Hello Rob,
> >
> > Hello.
> >
> > Do you mind if I move this conversation to the tcc mailing list (starting
> > with reposting my answer there?)
>
> Please do.

And I've cc'd this reply there too.

> > > I tried to use tcc for a project (in Delphi, using libtcc.dll), where
> > > I need dynamic compilation and multiple states.
> > > After some crashes, I started to study the source to find that there
> > > was almost no support for multiple states.
> >
> > What do you mean by "multiple states".  Do you mean one tcc.dll loaded
> > but multiple points of compilation progressing within the same project?
>
> Not compilation, just states (TCCState instances) with runnable code.
> I (dynamically) created several strings of C source code and wanted to
> have multiple functions doing some calculations.
> I called several times tcc_new (multiple states at the same time); at
> the end, the second call to tcc_delete raises AV (of course, because
> of global variables).
>
> Just try this:
>
> s1=tcc_new
> s2=tcc_new
> tcc_delete(s1)
> tcc_delete(s2)

Gotcha.

Ok, that seems to make sense...

> > > I took some time to find a solution to make it work. The goal was to
> > > make as less changes as possible; there could be some other solutions.
> > >
> > > I do not have right now a diff, and I'm not familiar with it. All my
> > > changes are prefixed with "gc".
> > > The reference is :
> > > tinycc-7909d3c7e712.zip
> >
> > Basically you run "diff -u oldfile newfile" and redirect the output to a
> > file. I can do it myself for the moment.
>
> I can easily do it, but I'm not used to read the output. It's much
> comfortable for me to use winmerge to inspect changes.

*shrug*  I don't own a windows machine, have never written code for windows, 
and don't plan to start.

There was one time an employer made me use a windows machine to write code on.  
It was back before Y2K, when Trilogy gave me an NT4 box.  I was writing Java 
code to be deployed on AIX.  I did not have administrator access to the 
machine (all the tools came preinstalled), yet over the next three months it 
slowly ate itself to the point where neither the compiler, the source control 
client, nor the app ran on the thing anymore.  I wound up using one of the 
AIX machines in the lab for the last few weeks, and didn't renew the 
contract...

> > > This is also inserted as comment in tcc.c, to describe changes:
> > > gc: changes for libtcc (NO significant changes for standalone version)
> >
> > Ideally I'd like the various versions to work the same.  If the global
> > state pointer has a short enough name, then the macros can go away and
> > the users can dereference them when they use them.  What's the overhead
> > of moving these globals into a struct?
>
> As I wrote, I wanted to keep as less changes as possible.

While I understand this impulse, the macros make what the code's actually 
doing non-obvious.

> Because compilation speed is listed as a main target, I thought we could
> keep global variables just as they were, because there is only one state
> anyway (in standalone version).
> The overhead is just extra dereferences, but I don't think it would be
> significant.

It's a separate cleanup and doesn't need to block this one.

> > I note that there's already a TCCState in tcc.h, why create another
> > structure rather than move stuff into that?  (There could easily be a
> > good reason, I'm just wondering what it is.)
>
> No other good reason than easily readable changes.

I'd prefer to have the most correct changes than the most minimal changes 
possible.

> > It seems like we could add some more structs: "backend" (for the code
> > generation backend) and "parser" (for the parser).  Probably TCCState can
> > be broken up, but I'm not sure along what lines.  (There's probably a
> > "control" struct covering things like the command line arguments that tcc
> > got that tell it how to behave.  Header and library search paths should
> > probably go there too...)  And then compilation state like the symbol
> > stack could go in a "compile" structure, perhaps?  (Or would "program" be
> > a better name?)
> >
> > The globals are a mess I've wanted to clean up for a while...
>
> Again, my first goal was to make it work with little changes.
> I also agree that they should be moved in TCCState, or better in a
> "sub-structure".

A sub-structure shouldn't have noticeable runtime overhead.  (Hmmm, I wonder 
if tcc will combine the offsets or do the math twice at runtime?)

> > > -move state dependent variables in a structure (TCCCompState) and make
> > > wrapping macros
> >
> > I already asked for more detail on what you mean by "state".  Possibly
> > this is the "compile" structure proposed above?  (Or "program".  Or some
> > better name. Here is what we remember about the code we've already seen.)
> >
> > > it does not make sense to do this for parser state, because it requires
> > > too many changes (macro conflicts with variable names)
> >
> > I prefer not to do macros at all, and just change the uses of the
> > variables. (Macros make it non-obvious what the code is doing, which is
> > bad.)
> >
> > And these changes can be made incrementally.
>
> I also don't want macros, but it was easier for me to apply changes
> which would not
> require modifications in any other source file.
> If it is generally accepted, of course it's better to refactor the source
> code.

One thing to keep the noise down is that we don't have to move _all_ the 
variables in one patch if we don't want to.  It naturally splits up by 
variable...

> > > -store this in TCCState as opaque
> >
> > Store what, a pointer to the new structure you added?
>
> Yes. Again, just to keep changes simple (in tcc.h)

Moving the new variables into TCCState definitely makes sense then.

> > > -allocate in tcc_new and release in tcc_delete
> >
> > um, ok...
> >
> > > -move included files lower
> >
> > I'm mixed about that.  On the one hand getting them out of the way is
> > good. On the other, it implies they need access to more variables than
> > they actually do...
>
> Yes, this was just needed because of moving variables in code.

Again, cleaning this up is a separate issue that doesn't need to be tangled 
with this change, so I'll worry about my objection later. :)

> > I'd really like to make it so that those aren't #included but are instead
> > separately compiled and linked, which is how multiple .c files normally
> > work...
> >
> > > -use "LIBTCC_STORAGE" to switch off "static" keyword
> >
> > Why?  (Details, please?)
>
> When they are inside the struct, you can't use "static". When they are not,
> static is good to avoid possible linker errors (same variables in other
> files). Otherwise why was "static" used?

General code hygiene, I think.

Moving them into a struct all the time seems the logical thing to do.

> > > as long as each state is "prepared" standalone (parse / relocate...),
> > > these should work:
> > > - calling dynamic code from several states
> > > - multiple state create/delete
> >
> > I'm still not quite understanding this...
>
> Basically I wanted this:
>
> s1=tcc_new
> tcc_compile_string(s1,...)
> tcc_get_symbol(s1, f1, "foo")
>
> s2=tcc_new
> tcc_compile_string(s2,...)
> tcc_get_symbol(s2, f2, "foo")
>
> ... use f1(), f2()
>
> tcc_delete(s1)
> tcc_delete(s2)

Hmmm...  So both compile instances have the same "foo" at the same time?

> Now it works, as long as they are "prepared" one at a time.
> The line
>     tcc_state = s1;
> in tcc_compile (and a few others) would work up to a point, but it's
> still not very clean.
>
> To make it very clean (and maybe even thread safe), you need to remove
> tcc_state completely
> and pass it to all functions.

I don't personally care about thread safety, but if somebody else does...

I'll try to take another look at this patch later today.

Rob
-- 
"One of my most productive days was throwing away 1000 lines of code."
  - Ken Thompson.