LADSoft / OrangeC

OrangeC Compiler And Tool Chain
http://ladsoft.tripod.com/orange_c_compiler.html
Other
284 stars 39 forks source link

Utils::AddExt not compatible with source files with more than one dot #1031

Closed rochus-keller closed 2 months ago

rochus-keller commented 2 months ago

My reference project includes a lot of files which include more than one dot, e.g. som.Vector.aa9eee315e.c Here is the project: awfy.zip

If I compile som.Vector.aa9eee315e.c with my orangefront and the -Y option, I just get one output file called som.Vector.aa9eee315e, which apparently is the icd version. The icf version is likely called the same and thus overwritten.

I traced the issue to Utils::AddExt, which does nothing in case of filenames like som.Vector.aa9eee315e.c; Utils::StripExt has worked correctly and left som.Vector.aa9eee315e, but then Utils::AddExt concludes that there is already an extension and does nothing.

I tried with this implementation instead:

void Utils::AddExt(char* buffer, const char* ext)
{
    char* pos = (char*)strrchr(buffer, '.');
    if (!pos || strcmp(pos,ext) != 0 )
        strcat(buffer, ext);
}

With this modification allone it doesn't work (instead it complains that it doesn't find file.c.C). To make it work, I also had to remove Utils::AddExt(buffer, ".C"); from line 567 in occparse.cpp.

With this modification, when I run with a single .c file, the .icf and .icd are generated correctly. But when I run with *.c, then all .icd files are generated, but an .icf is generated only for the first file (in my case Benchmark.icf). No indication of a problem from command line outputs.

So apparently I didn't get the logic yet. Any ideas?

rochus-keller commented 2 months ago

Meanwhile I found the reason why only one .icf is written if the command line says *.c. The problem seems to have been unrelated.

To fix it, I had to move and modify the if clause from line 812 of occparse.cpp to line 714, see https://github.com/rochus-keller/OrangeC/commit/45bb9032834cac2f7e1ceefff45d71a0ddccc817.

But there is still the question whether I considered all effects of the change to Utils::AddExt. There is e.g. the line 1021 in osutil.cpp which still states Utils::AddExt(buffer, ".c"); and which maybe has to be removed.

What do you think?

LADSoft commented 2 months ago

so i think occ does handle this somehow but it probably somewhere deals with it by looking for "..\" which is not what you want

I wasn't able to immediately see where it is taken care of but tomorrow I will look at it and figure it out.

Meanwhile that AddExt(buffer, ".C") is a bug, the C should be lower case because linux actually cares about case in filenames...

LADSoft commented 2 months ago

never mind i didn't read closely enough. Yeah there are a couple of bugs here. First one is the upper case C I mentioned,,, second one is this whole process of 'stripext/addext' breaks down when the two are used together and there are dots in the filename... an easy fix is to make another function 'replaceext' which does the two in tandom for those places in the code where it is required... I don't know if that is the best fix though. This is another one of those filename things that I'm probably going to have to redesign at some point...

GitMensch commented 2 months ago

Just as a note for a possible redesign: in GnuCOBOL we do it a bit different, creating all the names for intermediate and result file names up front. In doing so we check if the output name from the stage is specified by the user - if tes, then that's going out verbatim, if not then we build the base name from the input file once (so one stripext only) and strcat the expected file name extension. Later stages get the filename structure and use its entry.

... Just as an idea.

rochus-keller commented 2 months ago

Closed to continue in https://github.com/LADSoft/OrangeC/pull/1032.