Since some time I'm trying to separate the syntax from the semantics processing in the parser. It turned out to be quite complicated, so that I want to use some methodology. (Yes, I've been warned ;-)

For profiling and debugging I want to have both the old and new code in the same executable file, so that a commandline option or environment variable can be used to select which "version" to use. Is this feasable, WRT the invocation of fpc with e.g. the test suite? How would you do it?

Furthermore I have no clue how to use new I have to rebuild fpc, or how else is the fpc executable (or link?) related to concrete (machine/target specific) compilers?

The separation shall be done in multiple steps. In the first step I'll enclose all semantical code into begin-end, so that it can be easily hidden (folded) in the Lazarus IDE. Later these blocks shall be extended into

   if cond then
     new_proc
   else
   begin
     [old code, also copied into new_proc()]
   end;

This has the advantage over $IF... that the IDE allows to navigate to everything in both if-else branches. It also should allow to apply patches (from updated trunk), to some degree, because the code sequence is not much modified.

The new_proc procedures will be local subroutines first, which can share the local variables of the enclosing parse_... procedure. Later these subroutines will be moved into different units, where they finally are exchangable by changing the searchpath. Here again it's helpful to view both versions in different editor windows, side by side.

For first results see the dodi/parser_rewrite branch.

Perhaps I missed more issues? Any hints are welcome :-)

DoDi

Hans-Peter Diettrich wrote

In the first step I'll enclose all semantical code into begin-end, so that it can be easily hidden (folded) in the Lazarus IDE.

This is not needed -- you can use {$REGION} ... {$ENDREGION} directives instead.
If/when somebody implements http://bugs.freepascal.org/view.php?id=13974, you will be even able to auto-fold irrelevant parts automatically.

Hm, did you consider a proper object-oriented implementation? I know it is not easy, but since you are going for a great refactoring anyway...

For first results see the dodi/parser_rewrite branch.

Generally speaking, long-lived feature branches should be avoided in SVN, since the merging is incredibly painful. So it would be advisable to merge back as soon as possible, or even just work on trunk, using the techniques you mentioned above to avoid breakage.

Why? As long as the patches textually apply, it's no problem. But as soon as the changesets not longer apply the used vcs makes no difference.

I don't think this is a good idea.

My proposal is:
- make a proof of concept for a part of the parser, e.g. ptype.pas, then we profile the old and the new compiler with valgrind with code which stresses this part.
- then we decide if it's worth to continue

For first results see the dodi/parser_rewrite branch.

Please try to use an indention and coding style as the other compiler sources do.

I think also that a complete OOP approach should be used and: design it with multi threading in mind. Having a scanner/parser where multiple instances can be run at the same time would be a great benefit and I'am pretty sure that it will make it into fpc trunk.

Of course, this is the trivial case.

But as soon as the changesets not longer apply the used vcs makes no difference.

Well, you should really try using modern VCS ;-)
Anyway, I do not want to restart VCS flame yet again, so please let us forget other VCSes for now.

I just wanted to say that, in SVN, there is indeed a problem as soon as conflicts start to accumulate, so (in my experience) the optimal way is to avoid them
as much as possible by avoiding feature branches with a lifetime of more than a few days.
So my advice would be that Hans should work on trunk.

Well, I did, till the lazarus mercurial mirror repository broke. Yesterday, I tried to clone Graeme's git repository because I needed quickly fpc trunk: cloning aborted with some error message always at the same point. svn co was 1-2 min slower but it worked.

I don't see how this can be solved by another vcs.

I'll need begin-end anyways, when the code becomes a subroutine.

If/when somebody implements http://bugs.freepascal.org/view.php?id=13974, you will be even able to auto-fold irrelevant parts automatically.

That would be nice, indeed, but how should it know which if-else branch is irrelevant?

Yes, of course, it can be done in further steps. My very first approach used a semantics class, but after the performance discussion and other problems I dropped it. Some parser routines already are methods, of e.g. psub.TcgProcInfo.

I'd appreciate if my work would find its way into the trunk soon. But how to maintain the duplicate code (inlined and in procedures/methods)?

Include files were a technical solution, but only until the procedure/method code deserves further modifications. And I wouldn't like to see hundreds of parser include files...

DoDi

Works here! I used a new Ubuntu 10.04 install on my laptop.

$ git --version
git version 1.7.0.4

-------------------------------------------------------- $ git clone git://github.com/graemeg/freepascal.git fpc_github Initialized empty Git repository in /opt/git/fpc_github/.git/ remote: Counting objects: 139200, done.
remote: Compressing objects: 100% (31664/31664), done. remote: Total 139200 (delta 107465), reused 138515 (delta 106960) Receiving objects: 100% (139200/139200), 65.48 MiB | 265 KiB/s, done. Resolving deltas: 100% (107465/107465), done.
Checking out files: 100% (11821/11821), done.

--------------------------------------------------------

Above was the full history (including branches: trunk, cpstrnew, fixes) resulting in a 65MB download using the git protocol to access the repository.

If you don't need the full history, add the parameter: --depth 5 This will result in a history of only the last 5 commits of each branch & tag, and result in a much smaller download.

I can't comment on SubVersion, but with Git I have feature branches that live for a couple of months before they get merged. I have no problems with that. So maybe it's down to the way SVN is used? I don't know. All I can say (which applies to all VCSes), is that not all features (and thus feature branches) can be implemented and tested in a few days.

IMO we have to identify the critical parts of the parser first (performance wise), and benchmark otherwise identical code (not trunk vs. branch). I'm willing to spend some time with refactoring parts of the parser in various ways, until we can agree about an acceptable model.

One working model would be a SVN parser sub-project, so that multiple parser models could be developed and tested in parallel, with syncing reduced to the units in that project. The same could be done for an future threaded approach.

> For first results see the dodi/parser_rewrite branch. Please try to use an indention and coding style as the other compiler sources do.

For now I wanted to retain the original indentation, for best SVN merge compatibility. Since the indentation width differs even within single subroutines, I'd like to reformat all touched units, using the Lazarus JCF formatter, so that indentation will not be an issue any more.

I think also that a complete OOP approach should be used and: design it with multi threading in mind. Having a scanner/parser where multiple instances can be run at the same time would be a great benefit and I'am pretty sure that it will make it into fpc trunk.

An OO move can be done independently from any other refactoring. In a first step it should be sufficient to convert all current procedures into methods, and the global links into class members. All that is not related anyhow to my parser refactoring, but I can start such a new project on request. Once it has been accepted or rejected, I can restart or continue the parser refactoring accordingly.

Possibly the decision for a threaded model should be taken even before any refactoring in that direction, when threads require a very deep refactoring. I never wrote complex threads myself, dunno about the implications on all the related classes like nodes, procedures, modules, optimizer, code generators...

I'd recommend that some threading expert prepares such a refactoring, so that I can learn more about the specific coding requirements. The mere conversion of procedures into methods, and variables into fields or threadvars, won't help much, I fear.

DoDi

In our previous episode, Graeme Geldenhuys said:

This was all about the chance that patches don't merge in two heavily mutating branches.

Unless other systems really take into account the complete history of both divergating branches in the merge, and not simply patch/merge, I doubt VCS matters at all

Marco van de Voort wrote

Unless other systems really take into account the complete history of both divergating branches in the merge, and not simply patch/merge, I doubt VCS matters at all

Git's merge algorithm does this and is a lot more intelligent that SubVersion, and thus much less prone to conflicts. Git can even merge multiple branches at the same time into a single branch (known as octopus merge). Just a bit of useless information.

Hardy Heron:
$ git --version
git version 1.5.4.3
$ git clone http://github.com/graemeg/freepascal.git Initialized empty Git repository in /home/kla/freepascal/.git/ Getting alternates list for http://github.com/graemeg/freepascal.git Getting pack list for http://github.com/graemeg/freepascal.git Getting index for pack 20c6315fdc75fd363fdbb42cb7b9254de9b06fcc Getting index for pack bd6c15062406c8a956251541f46d453ce2ac0f6c Getting index for pack 1d6c282954cc60b46ebc1b605db21ac1aaff7dc5 Getting pack 20c6315fdc75fd363fdbb42cb7b9254de9b06fcc which contains 7b9867d805b3c1bc5cda8029bd2a40f9fe91546e error: Unable to get pack file
http://github.com/graemeg/freepascal.git/objects/pack/pack-20c6315fdc75fd363fdbb42cb7b9254de9b06fcc.pack transfer closed with 64356960 bytes remaining to read error: Unable to find 7b9867d805b3c1bc5cda8029bd2a40f9fe91546e under http://github.com/graemeg/freepascal.git
Cannot obtain needed object 7b9867d805b3c1bc5cda8029bd2a40f9fe91546e

This is out of my control.

Hi

If merging the rapidly diverging branches becomes a real problem, and if Git really can merge so much better (which I haven't compared much myself), you can always use a local Git repository, then commit and merge (actually rebase) there. Like:
git checkout DoDi_branch
... edit ... git commit
git rebase master

and then synchronize with git-svn link.

I am doing it with Lazarus development. See my instructions here: http://wiki.lazarus.freepascal.org/Lazarus_git-svn

I synchronize with trunk only.
You would need to configure git-svn link for 2 branches, trunk for pull-only, DoDi_branch for push.

Juha

Hi

Hardy Heron: $ git --version git version 1.5.4.3

OpenSuse 11.3:
$ git --version
git version 1.7.1

Works here. Time to update Ubuntu. :-)

Juha

Thanks for pointing to ptype.pas, these routines made me rethink the current refactoring :-)

While my first idea was to steer everything by the syntax, I now found this approach not very useful. Details like finding the proper location of errors (too many arguments...) suggest to use background information in the first place, not after the syntax has been verified.

Consequently the new refactoring goal should result in "building blocks", like building the *internal* representation of e.g. a formal (or actual) parameter list. The generalization or abstraction then would be the use of (language specific) parser rules, to obtain the name and attributes of every parameter in that list.

The primary benefit of this refactoring is the possible encapsulation of the related data structures, resulting in more freedom in the maintenance of the encapsulated details. At the same time it will be possible to exchange the syntactical parser, for further languages or also for dedicated modes, in case this allows to simplify or speed up compilation. The latter part can be postponed, or inlined "naturally" into the OO rewrite.

This new approach also will be much more robust than the preceding one, because it does not change the semantical code, nor does it require to maintain copies of that code.

I think also that a complete OOP approach should be used and: design it with multi threading in mind. Having a scanner/parser where multiple instances can be run at the same time would be a great benefit and I'am pretty sure that it will make it into fpc trunk.

With above redefinition we already agree about this (OOP) goal :-)

I'll retain a local backup of the parser_rewrite project, so that this project can be removed from the repository.

Then I'll start a new OO_rewrite project, and hope that it will be ready for a first profiling within a few days. Comments, anybody?

DoDi

That is weird, even via the http protocol I can clone the repository without problems. I canceled the clone after 9% - I have limited data allowance per month on my home internet connection. Have you tried cloning via the git:// protocol? The git:// protocol is more efficient and faster than http:// protocol.

-------------------------------------
$ git clone http://github.com/graemeg/freepascal.git fpc_html_github Initialized empty Git repository in /opt/git/fpc_html_github/.git/ remote: Counting objects: 139220, done.
remote: Compressing objects: 100% (31684/31684), done. Receiving objects: 9% (12602/139220), 3.57 MiB | 180 KiB/s ^C
-------------------------------------

Juha Manninen wrote

can always use a local Git repository, then commit and merge (actually rebase) there. Like:

When I started with Git, I actually did something like that with the tiOPF project. I used my existing SVN checkout directory. Then used git for local branching, by creating a git repository inside the existing svn checked out directory. Developed in the git branches, then merge back to git's master branch (which is equal to svn trunk), then do a normal 'svn commit'. The down-side is your svn commit is one large commit (you loose the branch history). Not to mention, I hate large commits. So now I simply use git-svn. But for other small apps or prototypes, I often create local git repository to manage my history while I try out some ideas.

I don't doubt the ability to merge, but will it result in usable/meaningfull code too ?

Marc (couldn't resist)

Only http(s) are possible due to firewall. It might be a proxy issue even. Anyways, svn works ;)

Marc Weustink wrote

I don't doubt the ability to merge, but will it result in usable/meaningfull code too ?

A silly question. Just take a look in the Git and Linux Kernel repository histories. Octopus Merges and normal 2-branch merges happen all the time, and in both projects the code works just fine afterwards.

[ Like a good net-citizen, I see we are getting off-topic. (beating Jonas to the punch) ;-) ]

Or you missed the contents of the question. The problem Marc brings up is: what happens if branch and trunk/another branch change the same line/tokens of code and then those branches are merged. A good vcs comes up with a conflict, a vcs trying to be too clever merges maybe both lines creating nonsense code or whatever.

(Well, and the Linux kernel isn't for me a good argument, it's quality decreased steadily during the last years.)

Florian Klämpfl wrote

is: what happens if branch and trunk/another branch change the same line/tokens of code and then those branches are merged. A good vcs comes up with a conflict, a vcs trying to be too clever merges maybe both lines creating nonsense code or whatever.

Of course git will also raise a conflict it that case. My point is that say you made a change in your branch, and in another branch that exact same change was also made, if you merge the two branches, git will resolve it without conflict because the code is identical in both branches. SubVersion is a bit brain-dead in that regard and will still raise a conflict - it apparently only looks at the line number, not the content of that line.

This was told to me by Vincent and Jonas a little while back, where I wanted to change the code in cpstrnew branch to match that of Trunk - but without using svnmerge. Even though the code would end up being identical, subversion raises a conflict. That sounds pretty stupid to me. In the end Jonas had to revert my commit, and resync the cpstrnew branch with Trunk - which now doesn't compile any more for me under Linux.

I never encountered this and doubt it :) because svn recognizes also things like the same change made in the repository and the wc and just ignores it. It's more likely that some changes regarding spaces was done.

Anyways, why do you want to do the same change twice? Just make it once and merge.

This was told to me by Vincent and Jonas a little while back, where I wanted to change the code in cpstrnew branch to match that of Trunk - but without

Why would one do so? This causes only confusion because nobody can reconstruct what has been merged so far.

using svnmerge.

We don't use svnmerge anymore but native svn merge because svn has a good merge tracking for several years now.

Well, I would have expected this. trunk changed a lot since cpstrnew was created.

Florian Klaempfl wrote

I never encountered this and doubt it :) because svn recognizes also

Then some core team members should retrain themselves with SubVersion so they know what it can and cannot do. This will reduced them spreading incorrect information to others not so proficient with SubVersion. I don't used SubVersion for over a year now, so I've lost some of my SVN knowledge, or what the latest svn can and cannot do.

Anyways, why do you want to do the same change  twice? Just make it once and merge.

I was trying to compile the cpstrnew branch with a much newer Trunk compiler, to finally submit some of my local changes. The compiler complained about certain language changes and code syntax. I don't track trunk changes, so simply fixed the cpstrnew branch to compile again - without looking at trunk for answers. Vincent raise the issue that my changes might cause merge conflicts and pointed me to the Trunk code changes. I suggested to simply change the mentioned code to match trunk and be done with it (manual code change - I don't know how to merge with svn, or if I can only merge a certain patch. eg: like Git's cherry-pick feature). Again I was told I cannot simply match the code, because even though it would be identical to trunk, it will still cause merge conflicts. Jonas then re-synced cpstrnew branch with Trunk - now it doesn't compile anymore. I don't know how or under which platform Jonas tested the cpstrnew branch after he did the re-sync.

Florian Klaempfl wrote

Anyways, why do you want to do the same change twice? Just make it once and merge.

In a distributed environment this is more common, like: person B partially pulls (some commits) from person A, and person C pulls from both A and B. Or a diamond where B and C pull from A, and D pulls from B and C.

Micha

Yes, but those are merges of regular commits?

See below.

If you adapted the code without looking at trunk it is *very* likely that you didn't create exactly the same code as in trunk so Vincent is right: this is very prone to conflicts. One space less and it is a conflict for all vcs in the world (hopefully).

True, but the point is that in both cases it's possible that someone merges a specific commit twice. Maybe subversion also handles this case well; I have never tried.

One may get the same problem when merging two-way which is necessary with long-lived branches (the branch merges commits from trunk, and later the complete branch is merged back into trunk). Or those branches have to 're-based' (not a real subversion action is it?) but then you can't merge from them anymore.

Micha

If merge tracking doesn't prevent the merge, subversion ignores changes being already in a certain file, at least to all of my experience. I think Graeme simply misunderstood Vincent's concerns, see my other mail.

Graeme Geldenhuys wrote

This was told to me by Vincent and Jonas a little while back, where I wanted to change the code in cpstrnew branch to match that of Trunk - but without using svnmerge. Even though the code would end up being identical, subversion raises a conflict.

That's incorrect. The problem was that while your changes were functionally identical to the changes in trunk, they were not syntactically identical. I don't know of any vcs that can analyse patches and conclude that even though they are textually different, they actually have the same effect on the program flow.

As Florian mentioned, if the changes are textually identical, then svn will not have any problems merging later on.

In the end Jonas had to revert my commit, and resync the cpstrnew branch with Trunk - which now doesn't compile any more for me under Linux.

What is the error?

Jonas

In our previous episode, Graeme Geldenhuys said:

There is a difference between

- two way merge (patch and diff)

- a three point merge (cur revision of both
branches and the ancestor file of both branches), which afaik SVN does.

- a n-point merge that takes tracks through each revision of both branches, what according to Graeme GIT does.

Florian Klaempfl wrote

If you adapted the code without looking at trunk it is *very* likely that you didn't create exactly the same code as in trunk so Vincent is right: this is very prone to conflicts. One space less and it is a conflict for all vcs in the world (hopefully).

This was indeed the case which Vincent commented on. I then suggested to make another commit in cpstrnew to make the code syntactically identical to Trunk. I was told this would make no difference, SubVersion would still raise a conflict. So instead of me making another commit, Jonas re-synced the whole branch.

Jonas Maebe wrote

That's incorrect. The problem was that while your changes were functionally identical to the changes in trunk, they were not syntactically identical.

See my reply to Florian. My idea was to add another commit to make it syntactically identical, but was told this would make no difference to SubVersion - it would still raise a conflict at a later date. I'll create a SubVersion sandbox repository so I can play with the latest SVN and see how things work now.

What is the error?

I can't test now, I'm in the process of rebuilding my laptop. As soon as I have a running system again, I'll raise the issue in a new thread. [Something I should have done right after you re-synced the branch, unfortunately I didn't have time right then.]