LADSoft / OrangeC

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

Support base file names with dots and icf output for all provided source files (not only one) #1032

Closed rochus-keller closed 1 month ago

rochus-keller commented 2 months ago

This is the continuation of https://github.com/LADSoft/OrangeC/issues/1031 which is hereby closed.

I checked where the functions using Utils::AddExt are used and came to the conclusion that my modification of Utils::AddExt works up to the few cases where a default suffix should be added when the user provides a filename without suffix. But from my humble point of view this case should rather be treated as an error; it might have been a decent behaviour as long as Orange only was a C compiler, but today a plethora of possible suffices is expected, so it doesn't make sense to just arbitrarily select one.

Conclusion: the compiler doesn't support filenames provided by the user without suffix anymore, and doesn't change the suffix of a user provided filename (as also suggested by GitMensch). And the generation of output filenames by chopping the suffix and adding a new one is supported.

I also modified the code order in occparse so the icf output is in the loop through all provided source files (the original version instead wrote the icf of the last file with the name of the first file).

LADSoft commented 1 month ago

thanks for looking at this!

as some history, i was originally copying the old borland compiler.... back in the day when C++ was first coming out it still appended the .c automatically. I never revisited that behavior.

It looks today that most major compilers pass files without extensions on to the linker.... which then can make some assumption on its own about the nature of the file. on windows CLANG and MSVC assume it needs a .obj extension, gcc thinks it is a linker script.

I'm ok with just bailing for such files instead of making assumptions about them, even thought it could conceivably break make files, but there really ought to be some error if we do it... we shouldn't just be blithely ignoring it. If that error comes from trying to compile it directly as specified that is fine I guess.... we just need something to say that this file couldn't be processed as a source file...

rochus-keller commented 1 month ago

Ok, I see. As far as I understand the code in occparse main, a file with no extension would just implicitly be treated as a C file; without my modifications, a file without extension was just given the extension ".C", but there is no check on this extension anywhere, so apparently a C file is assumed by default (with or without the .C extension). I will check in the debugger what actually happens with my modifications if a file has no extension.

rochus-keller commented 1 month ago

Meanwhile I conducted tests on Windows 11 and Debian 12 x86 with the most recent commit posted here (c095451).

The frontend behaves as assumed.

If I pass a file without extension, it is assumed to be C and compiled as such. The generated icd and icf look decently.

If I pass the filename without extension, but the actual file has a .c extension, the frontend complains that it cannot find the file with the passed filename. And vice versa (passing with extension but the file on disk has none).

So from my point of view the modifications already meet your requirements stated above.

LADSoft commented 1 month ago

thank you for testing it!

that is fine, i don't mind it not being able to compile it i just want to know it didn't compile it lol.... I will accept these changes later on :)

LADSoft commented 1 month ago

:so i started looking closer and the windows smoke test failed; here are the offending lines out of the build log:

    occ /c /! /DSTD_NEWHANDLER /DWIN32_LEAN_AND_MEAN -o.\msvcrdm.o msvcrdm.c
    occ /c /! /DSTD_NEWHANDLER /DWIN32_LEAN_AND_MEAN -Wcm -o.\c0.o ..\pe\c0.c
    del msvcrt.l
Could Not Find D:\a\OrangeC\OrangeC\src\clibs\platform\WIN32\msvcrdm\msvcrt.l
    olib -! --noexports msvcrt.l +- .\c0.o .\msvcrdm.o
Error: Module '.\c0.o' does not exist
Error: Module '.\msvcrdm.o' does not exist
Error while reading input files

so apparently having .\ in the path is failing for some reason... can you take a closer look?

rochus-keller commented 1 month ago

Changing code that I don't know and where I can't estimate the full effect of the change is always a challenge, especially since I only have a subset of the code in my development environment.

I have a theory as to what the problem might be and have uploaded a new commit. Actually there was a section in osutil outputfile the purpose of which I didn't understand (and placed a TODO in the comment), but now I think that's the place where it fails. I can imagine that in the build the output file is specified without extension, and therefore it does not work because I had commented out the part. But that's just a guess, as I can't test it completely on my end.

LADSoft commented 1 month ago

lol i guess i understand your frustration, there are a lot of unknown dependencies in this source code...

since i really want the dots thing to be fixed, could you revert your changes from this morning (you can use a force push if you need to) and I will figure out what the problem is and fix it?

rochus-keller commented 1 month ago

No frustration, just stubbornness ;-)

Unfortunately even now when I revert to the original suffix logic (329342d) the windows build fails. It started to fail with abb3393, but since I reverted this (besides the .c instead of .C), I would have assumed that it should work.

Unfortunately, I'm not such a great git wizard, so I'd better leave it with the history change before I break something.

LADSoft commented 1 month ago

ok lol yeah first time i did a force i had to be told exactly how to make it work lol...

so im gonna ignore this commit because there is some confusing stuff going on, but will make this change in master:

#ifndef ORANGE_NO_MSIL
        case ARCHITECTURE_MSIL:
            msil_examine_icode(head);
            break;
#endif

I looked at your other changes again and the 'compile to file' change in occparse.cpp wouldn't have worked so well for me since the optimizer and backend expect everything specified on the command line in one file, maybe I will look at making an ifdef change for it though since it seems like it could be a useful thing in some situations....

rochus-keller commented 1 month ago

I still find it very strange why the windows build still fails even if I effectively reverted to 45bb903, which was the last commit the windows build worked.

Interestingly the windows build failed after 30 minutes (compared to the 5 minutes before my last commit). Maybe I'll be able to make sense of the huge amount of output from the build log and find a clue to the answer.

I would find it very desirable if my version of the frontend does not evolve away from yours, since it essentially does the same thing (except that I only want to compile a subset) and I dock my special code to the icf file. I could even live with all input files going into the same icf.

rochus-keller commented 1 month ago

I close this and start over in a separate branch, with the new define up-front and minimal changes.

See https://github.com/LADSoft/OrangeC/pull/1033 for continuation.

GitMensch commented 1 month ago

Nearly exactly 30 minutes may be a build timeout from Appveyor, you'd see that in the end of the build log.

rochus-keller commented 1 month ago

Unfortunately I don't know Appveyor, but I have now tried with a new, minimal pull request with only a few new ifndefs and effectively no change to the windows build and it stops after 30 minutes, just stating "Error: The operation was canceled."