Hans-Peter Diettrich wrote

Now I hope that this patch will be applied to the trunk soon.

No. It's a mess and I won't sort it out. Not to mention the indentation style not following the style used in the surrounding code, comments I made about the older patch are ignored, strange changes without any comment like in cfidwarf etc.

In this new approach I moved some global variables into classes, omitting variables that require to many consequential changes to the remaining compiler code - see patch #17584. This patch will allow for easier implementation of parallel processing in the compiler, or for multiple front-ends.

The variables have been replaced by (getter) functions, with some added procedures for updating their values - this is where getter/setter pairs are inapplicable for structured types, or require too many consequential changes for using pointers instead of plain variables. If somebody can suggest a less intrusive solution, more global variables can be moved.

Now I hope that this patch will be applied to the trunk soon. It's a very big one, including some previously supplied patches. These mandatory patches already have been included, because I found it not so easy to merge these together. Maybe my fault...

As already mentioned, I'll try to implement further front-ends now. This work will be based on this patch, and result in changes to almost all parser units. This time I'll try to implement the required changes in just another front-end, so that both the old and new parser can be tested and compared easily. If there is common interest in this project, I can open another branch for it, where I (hopefully) can avoid the problems with my preceding branches.

DoDi

Would you consider turning those structured types into objects?

Properties with getter/setter pairs would make tracing (placing a breakpoint in the getter/sertter would help identify the caller) easier.

Cheers,

Adem

2010/10/9 Adem :

Would you consider turning those structured types into objects? Properties with getter/setter pairs would make tracing (placing a breakpoint in the getter/sertter would help identify the caller) easier.

The only problem with that line of thinking is that every time your are calling that variable you are adding the extra steps. This is impeding on performance of both compiling and maybe even execution.

Java people love getters and setters. :-) I think namespaces are in order though. A place to group variables (const and var, etc) though. This way we can just point to this mechanism in stead of having class creep.

Sigh... Units are namespaces, since 25 years.

Michael.

Why would globals be called so often as to impact performance?

LOL. I was talking about another way similar to c# namespaces.

This solution has some limitations. The conversion of a packed record requires much coding, the fields can not be used as var parameters and cannot be inc'ed etc. And then there is the time required to create and destroy the objects. In former attempts I converted the getters to return pointers instead of values, to overcome these problems, but then *every* usage of such a variable has to be modified accordingly - what I wanted to avoid this time.

Properties with getter/setter pairs would make tracing (placing a breakpoint in the getter/sertter would help identify the caller) easier.

Static analysis is easier with different names for the "setters". In many cases specialized setters can replace more than a single statement.

DoDi

Inlining the getters should not add much overhead. I don't feel that compilation is slower with the moved variables, on my x86 box, but perhaps somebody can benchmark that more precisely, and on other architectures.

DoDi

> Now I hope that this patch will be applied to the trunk soon. No. It's a mess and I won't sort it out.

What do you feel a need for sorting out?

Not to mention the indentation style not following the style used in the surrounding code,

The new or changed parts can be identified by their different naming style, more Pascal like, with not everything lowercase. The same for indentation, where different styles are already found in the source code. Refactoring, and maintenance in general, is much easier with more condensed code, without excessive indentation and almost-empty lines.

Provided that there exists a defined indentation style, a formatter tool can transform any code into the defined format. In so far I cannot understand according complaints :-(

comments I made about the older patch are ignored,

Which in detail do you mean?

strange changes without any comment like in cfidwarf etc.

Ooops, cfidwarf should not have been affected. I guess it's due to the different base revision in my merge :-(

DoDi

Splitting the patch in understandable parts which can be committed separately with appropriate commit messages, fix formatting, remove unneeded parts, remove stuff to be discussed first, see below.

It is good practive to patch and commit sources in well defined, traceble steps and this is not possible with this patch.

As I said, we try to use the always a coding style like the surrounding code.

Refactoring, and maintenance in general, is much easier with more condensed code, without excessive indentation and almost-empty lines.

This is your opinion.

> comments I made about the older patch are ignored, Which in detail do you mean?

The aasmbase change, see my comment in your issue report.

This is part of what I mean with mess.

I.e. one patch for every single moved variable???

Some separatable patches have been presented already. I merged them into the big patch, to simplify merging with trunk. Feel free to apply these patches first...

>> comments I made about the older patch are ignored, > Which in detail do you mean? The aasmbase change, see my comment in your issue report.

Question: do you have a better solution for the ppudump dependencies?

Okay, I'll stop rebasing my patches to the head revision.

DoDi

If needed, yes. If one variable is moved and it involves more files, it needs a separate commit. It makes also things like bisecting bugs easier.

Did you ever work for a longer time in a team with a vcs?

Feel free to apply these patches first...

This another part of the mess.

There is nothing wrong with using aasmbase in ppudump.

So you already found bugs in my contribution?

Did you ever work for a longer time in a team with a vcs?

Yes.

> Feel free to apply these > patches first... This another part of the mess.

Ah, I see. My work is not appreciated at all, and now it is rejected for almost formal reasons :-(

> Question: do you have a better solution for the ppudump dependencies? There is nothing wrong with using aasmbase in ppudump.

Using aasmbase will introduce unwanted dependencies later, occuring in the used units.

Simple question: would you apply my patches immediately, if they *only* would meet the expected granularity and coding style?

DoDi

Yes. The change in cfidwarf reverted a bug fix of a few days ago.

Indeed, also formalisms are a reason to reject a patch.

No. It does not. aasmbase depends only on a very few basic compiler (and rtl units).

No patch will be applied immediatly and only fullfilling basic coding requirements is needed but not sufficient.

In our previous episode, Andrew Brunner said:

LOL. I was talking about another way similar to c# namespaces.

Do you have some proposal somewhere? I never understood how this can be added to FPC (or Delphi) in a workable way.