LADSoft / OrangeC

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

Adding #include_next for the sake of sanity in regards to libcxx #523

Closed chuggafan closed 3 years ago

chuggafan commented 4 years ago

While #include_next is not officially supported by the C standard, it would be useful to implement if only so that libcxx can be truly dropped in so that every time we update a libcxx version (in the future) we don't have to manually go in and delete files that have this. https://gcc.gnu.org/onlinedocs/cpp/Wrapper-Headers.html

This is for my future sanity, and it would get rid of a ton of the startup editing work in relation to the #355 stuff.

I understand why they have it there, but man is it annoying to deal with.

GitMensch commented 4 years ago

I totally agree that:

LADSoft commented 4 years ago

yeah i saw that but it didn't seem used too often at least in what I had started compiling... it does make sense to do it though. The only reason I didn't do it immediately is it isn't quite straightforward given the current design of the compiler...

chuggafan commented 4 years ago

It gets used in system headers near-exclusively, I tried to take a look at the pre-processor (which is where this would take place) but I have no clue how to modify SrchPath in https://github.com/LADSoft/OrangeC/blob/5acfc755b241a320751b8d67a1e1cf97549f20f5/src/ocpp/ppInclude.cpp#L229 here to actually do it, I think a way this can be done is by skipping the first time it finds a file successfully and then continue chomping, now that I'm taking a 2nd look at it, I think I can have a PR when I get started working on it to support this "feature".

However: as a warning, this "feature" is EXTREMELY dependent on the order of search paths in the includes, so we may have to modify how that works so that the compiler in c++ mode first looks at a list of C++ headers, THEN a list of C headers, and can no longer just dump everything into a generic /include/ folder.

chuggafan commented 4 years ago

Update: I've gotten decently far on this to get it to work, my only issue seems to be something with RetrievePath only giving us one option before kicking us out of the paths that are available or something of the sort? I can test a lot more sometime next week rather than this week, but I should have the PR fully tested and finished along with implementing __has_include_next (which takes 5 seconds extra of work) along with it, with associated tests (hopefully). This will be a good first step to just dropping in libcxx to upgrade in the future.

LADSoft commented 4 years ago

yeah retrieve path might modify the incoming path list... should be fixable though.

Thanks! I'm thinking of starting to fix some of the smaller items in the milestone tonite...

LADSoft commented 4 years ago

we can change where the includes are located, might be a bit of a challenge to get the windows installer to clean up properly though in case of a reinstall...

chuggafan commented 4 years ago

I'm going to be honest at this point after checking what's going on in RetrievePath that I have no clue what's going on, so I'm going to strip all the debug information tomorrow and throw up a PR for you to review what's going on, cuz the test setup I have setup just doesn't make any sense to me.

But somehow it's getting the libcxx stuff to stop whining at me, and I'm just tired of having to figure out.

LADSoft commented 4 years ago

I went ahead and merged the pull to prevent it getting too stale; will leave this open until we get both the open issues closed. Open issues are:

1) crash when trying to include_next beyond the end of the include list 2) ocpp skips the first item in the path

chuggafan commented 4 years ago

Sorry about letting the PR get stale, I'll get to work on testing the fix I made about a week ago for the crash later tonight since I finally get released from the pain of the week today.

LADSoft commented 4 years ago

I was wondering if you had a chance to look at testing your fix for the crash? No particular hurry though.

chuggafan commented 4 years ago

Turns out I had to do more assembly debugging over the weekend than I thought, and this week turns out to have even more problems than the last in many ways, so overall I have less time this week than the last in terms of doing this, however I think I can sneak some stuff in since I finally retarted my computer so that the AV is not longer locking up VSCode installations cuz I found the time for that yesterday.

LADSoft commented 4 years ago

any news on this?

chuggafan commented 4 years ago

Still have been getting very little time compared to during the summer, should be able to start work in earnest in about 3 weeks after everything with what I'm doing is done. I've mostly dropped focus on this entirely as I've had a whole slew of problems that prevented me from doing any work on this

LADSoft commented 4 years ago

ok sounds good!

LADSoft commented 4 years ago

if it is ok with you I may fix this myself sometime next week... I'm nearly to the point where I want to rearrange the header file directories :smile:

chuggafan commented 4 years ago

Go right ahead, I am still setting up a new environment atm and everything should be cleaned up by next week.

chuggafan commented 3 years ago

I've begun looking into this again, there's a "fix" that works that's about... 4 lines long, however this makes it so it does not skip filenames within the same path as the system search path....

I think a proper fix looking at this situation in particular and the way this is interacting is to actually change how the preprocessor does it's file directories into a list system where we traverse the list one by one, starting from highest priority -> lowest priority and use whatever is the highest next priority first.

I tried replicating this but I'm not quite sure on the solution anymore, this works for everything except if the "bad" version is in the system path, which is the common use case.... ugh.

On second thoughts: I can do this the way it's currently set up, I would just have to modify the starting position and system include paths, or I could do a thing where if the thing is true, the same include path as the file is skipped... so many things and my brain has lost a lot of the information from when I was first working on this.

LADSoft commented 3 years ago

I thought about this and I think you could do it with two lists, one for system paths and one for user paths. They would both basically be a concatenation of the user and system include paths.

Two lists would be required because the user-specified include paths are searched in a different order depending on whether you specify it as a system include or a user include in the #include statement... and care has to be taken for special paths like the current directory and the path the current include file is found in.

The lists would be created basically from the same ordering found in ppInclude.cpp::FindFile. They could be created any time after command line processing is completed.

You could choose the list and the position within it based on the last #include statement, then skip special directories when traversing it for purposes of using_next...

chuggafan commented 3 years ago

The way GCC and clang does it is with one list for user-based paths and one for system, as you traverse the include path you go down the list, the list starts off with the current directory, then user includes, then system includes, this is true for include_next in particular, since it does not care about < > or " ".

The way I was thinking I can modify this setup is by making pushFile accept an error as a non-file, then when popped it just auto-errors and says that this file doesn't exist atm.

As I said, the reason this currently breaks is that the SrchPath functionality currently does not have a way to travel errors up. A redesign to fix this is necessary either way, the question is if we want to switch the behavior to the GCC functionality or not?

No matter what, there's a redesign on our hands, the question is whether I'll be comfortable enough around the codebase in the time I have to work on it to create such a change, the former one I'm 100% sure I'm not familiar enough yet, the latter I'm confident is possible for me.

LADSoft commented 3 years ago

I guess try the one you are comfortable with. If we ever need it to work the way GCC does we can refactor again later :smile:

chuggafan commented 3 years ago

Turns out I was overcomplicating the problem, what I really needed to do what implement a way to force ourself to return if we are skipping directories and we include things from the same directory.

(P.S: Being able to #include_next in the first place in a top-level file should actually be an error, but I have no idea how to check for this at this point in time, I do now know how to solve that problem you were having, however.

LADSoft commented 3 years ago

so the #include_next seems to not work, here is what I did.

I copied the libcxx headers (includeing the C-style headers) into \orangec\include, and copied the c headers into \orangec\include\c. Then I modified \orangec\bin\occ.cfg as follows:

"-z%ORANGEC%\include;%ORANGEC%\include\c"

which sets up the system include path. I also modified the compiler to #define the _need* macros needed by the libcxx c-style headers.

now when compiling the compiler finds the libcxx header, but says it cannot include_next when the libcxx header tries to include_next the c-style header. Am I doing something wrong here?

chuggafan commented 3 years ago

If you've got a branch that does this easily, (e.g. via makefile), can I get that pushed so I can investigate? It seems like it SHOULD work based on the idea of it going from first to last but it might be my mistake. The way this works is 100% based on the order of includes, so if \include is after \include\c it won't work.

LADSoft commented 3 years ago

@chuggafan - i checked the current work in on the libcxx branch. To use it, run the clx10_include_next.bat file to readjust the contents of the include directories and load the correct occ.cfg into \orangec\bin. (this branch already has the libcxx 10 sources replacing the libcxx 8 ones),

To see the problem there are sample files you can try to compile in \orangec\src\occparse\temp that will showcase it. Note that adding ';' on the end of the system include path (in occ.cfg) might be a fix to the problem but then there are other problems - i'm not sure if they are related to the include_next at this point or not. There is a potential there is some unrelated compiler bug although I thought I had fixed that one...

later if you want to switch back to the main branch do the switch first then use the 'clx.bat' file to switch the include files back to libcxx 8.

chuggafan commented 3 years ago

Where is \libcxx 10\ gotten from for this test so that I can use it? Edit: oh right, not needed

I am getting a compile error though, so I'll investigate why...

chuggafan commented 3 years ago

I just found one: We lack __builtin_strcmp so anything that compiles using the typeinfo header dies immediately, and I am going to focus on occparse rn, so that should get shoved into the "We know this atm, but this is a separate issue to be solved".

chuggafan commented 3 years ago

Figured out the root cause of the issue you were describing: the way I did it, when I do #include that scans from the start, it doesn't reset the traverse directory relative to that for that specific things include.

chuggafan commented 3 years ago

Fixed it on my end, it was that I didn't account for there being a nullptr for a path while also having a buffer, this had to do with hitting the end of the file buffer before doing stuff. Whoops, my fault.

LADSoft commented 3 years ago

ok that fixed it. Thanks much!

LADSoft commented 3 years ago

ok so that fixed that issue, but now I have another one lol. if I do the same test, with the files I gave you,

occ t1.cpp works

but occ /IHELLO t1.cpp gives me a bunch of undefined symbol errors.

chuggafan commented 3 years ago

Having taken a look, without /IHELLO, this is actually just a fault of include directories, because the file support/win32/locale.h does not exist in the include directory (at least for me), so the local locale information such as LC_COLLATE_MASK or LC_CTYPE_MASK is completely missing.

LADSoft commented 3 years ago

you should be trying it on the libcxx branch with the libcxx executables. You won't necessarily see the same issues on the master branch. For me, when everything is set up properly it works fine if I don't specify a /I switch, and doesn't work if I specify a /I switch. Here is what I get when specifying a /I switch:

occ Version 6.0.45.1 Copyright (C) LADSoft 2006-2020
Error( 18)    \orangec\include\cstring(69):  Undefined symbol 'memcpy'
Error( 18)    \orangec\include\cstring(70):  Undefined symbol 'memmove'
Error( 18)    \orangec\include\cstring(71):  Undefined symbol 'strcpy'
Error( 18)    \orangec\include\cstring(72):  Undefined symbol 'strncpy'
Error( 18)    \orangec\include\cstring(73):  Undefined symbol 'strcat'
Error( 18)    \orangec\include\cstring(74):  Undefined symbol 'strncat'
Error( 18)    \orangec\include\cstring(75):  Undefined symbol 'memcmp'
Error( 18)    \orangec\include\cstring(76):  Undefined symbol 'strcmp'
Error( 18)    \orangec\include\cstring(77):  Undefined symbol 'strncmp'
Error( 18)    \orangec\include\cstring(78):  Undefined symbol 'strcoll'
Error( 18)    \orangec\include\cstring(79):  Undefined symbol 'strxfrm'
Error( 18)    \orangec\include\cstring(80):  Undefined symbol 'memchr'
Error( 18)    \orangec\include\cstring(81):  Undefined symbol 'strchr'
Error( 18)    \orangec\include\cstring(82):  Undefined symbol 'strcspn'
Error( 18)    \orangec\include\cstring(83):  Undefined symbol 'strpbrk'
Error( 18)    \orangec\include\cstring(84):  Undefined symbol 'strrchr'
Error( 18)    \orangec\include\cstring(85):  Undefined symbol 'strspn'
Error( 18)    \orangec\include\cstring(86):  Undefined symbol 'strstr'
Error( 18)    \orangec\include\cstring(88):  Undefined symbol 'strtok'
Error( 18)    \orangec\include\cstring(90):  Undefined symbol 'memset'
Error( 18)    \orangec\include\cstring(92):  Undefined symbol 'strlen'
Error( 18)    \orangec\include\cstdint(152):  Undefined symbol 'int8_t'
Error( 18)    \orangec\include\cstdint(153):  Undefined symbol 'int16_t'
Error( 18)    \orangec\include\cstdint(154):  Undefined symbol 'int32_t'
Error( 18)    \orangec\include\cstdint(155):  Undefined symbol 'int64_t'
Error(  1)    \orangec\include\cstdint(155):  Too many errors or warnings
chuggafan commented 3 years ago

Well, looking at this via OCPP didn't help:

Error C:\OrangeC\include\__config(513): Expected ')'
Error C:\OrangeC\include\__config(517): Expected ')'
Error C:\OrangeC\include\__config(521): Expected ')'
Error C:\OrangeC\include\__config(531): Expected ')'
Error C:\OrangeC\include\__config(535): Expected ')'
Error C:\OrangeC\include\__config(536): Expected ')'
Error C:\OrangeC\include\__config(543): Expected ')'
Error C:\OrangeC\include\__config(547): Expected ')'
Error C:\OrangeC\include\__config(551): Expected ')'
Error C:\OrangeC\include\__config(564): Expected ')'
Error C:\OrangeC\include\__config(568): Expected ')'
Error C:\OrangeC\include\__config(572): Expected ')'
Error C:\OrangeC\include\__config(1458): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\typeinfo(2): Error directive: Must use C++ for typeinfo.h
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\typeinfo(2): Error directive: Must use C++ for typeinfo.h
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\memory(685): Expected ')'
Error C:\OrangeC\include\memory(697): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\typeinfo(2): Error directive: Must use C++ for typeinfo.h
Error C:\OrangeC\include\memory(685): Expected ')'
Error C:\OrangeC\include\memory(697): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\memory(685): Expected ')'
Error C:\OrangeC\include\memory(697): Expected ')'
Error C:\OrangeC\include\memory(685): Expected ')'
Error C:\OrangeC\include\memory(697): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\memory(685): Expected ')'
Error C:\OrangeC\include\memory(697): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\type_traits(2679): Expected ')'
Error C:\OrangeC\include\__threading_support(123): Error directive: Unsupported architecture
Error C:\OrangeC\include\memory(685): Expected ')'
Error C:\OrangeC\include\memory(697): Expected ')'
Error C:\OrangeC\include\memory(685): Expected ')'
Error C:\OrangeC\include\memory(697): Expected ')'
Error C:\OrangeC\include\memory(685): Expected ')'
Error C:\OrangeC\include\memory(697): Expected ')'
Warning C:\OrangeC\include\__undef_macros(17): Warning directive:: macro min is incompatible with C++.  #undefing min
Warning C:\OrangeC\include\__undef_macros(29): Warning directive:: macro max is incompatible with C++.  #undefing max
Error C:\OrangeC\include\bitset(275): Error directive: This constructor has not been ported to this platform

OCPP has this problem, but OCCPARSE doesn't. ?????????? Does this have to do with __has_feature?

Also: I've confirmed this is a bug somewhere in the stuff by dumping occparse include data, for some reason it didn't get included? I'm investigating as I write this.

chuggafan commented 3 years ago

I've now confirmed this is a problem with my implementation code, I THINK it has something to do with not resetting the directories when doing include_next depths, will update later

chuggafan commented 3 years ago

I've figured out the bug: When doing system searches, we ignore the user based include paths, so officially, those "never get skipped", so I have to do a way to count the number of include paths, presumably, due to our parsing of include paths, I can just do the number of semicolons + 1 to do this.

LADSoft commented 3 years ago

there seems to be something wrong with ocpp... it doesn't work at all right now. Let's not worry about that though, maybe I'm still feeding it the file paths incorrectly? I can look into that separately...

I hate to keep this going but now OCC works if you add a /I switch, but gives this if you don't:

occ Version 6.0.45.1 Copyright (C) LADSoft 2006-2020 Error \orangec\include\float.h(79): Could not open using #include_next for input Error \orangec\include\wchar.h(16): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error( 77) \orangec\include\initializer_list(11): Too many identifiers in declaration Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\string.h(60): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\stdint.h(123): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\stdio.h(16): Could not open using #include_next for input Error \orangec\include\ctype.h(38): Could not open using #include_next for input Error \orangec\include\wctype.h(53): Could not open using #include_next for input Error \orangec\include\wchar.h(16): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\limits.h(47): Could not open using #include_next for input Error \orangec\include\errno.h(31): Could not open using #include_next for input Error \orangec\include\locale.h(42): Could not open using #include_next for input Error \orangec\include\stdio.h(16): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\stdlib.h(20): Could not open using #include_next for input Error \orangec\include\wchar.h(16): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error \orangec\include\stdlib.h(20): Could not open using #include_next for input Error \orangec\include\stdlib.h(20): Could not open using #include_next for input Error \orangec\include\stddef.h(17): Could not open using #include_next for input Error( 11) t1.cpp(8): Syntax error: } expected 3 Errors

chuggafan commented 3 years ago

Damnit. I'll figure this out in the next ten minutes.

LADSoft commented 3 years ago

hehehe I fixed ocpp now. Off to test your latest change :)

LADSoft commented 3 years ago

this seems good now :smile:

LADSoft commented 3 years ago

ok another bug, see 'complex.h'. In the current library, one exists in both the \orangec\include and the orangec\include\c directories. The first does a #include_next on the second. The second one never gets included. Simply renaming the second one and its reference in the first one fixes the problem. For example compile the file \orangec\include\clibs\cmath\cgamma.c to see the problem.

chuggafan commented 3 years ago

I thought that I fixed this? Do you have a command line argument I can use to just raw compile this for occ so that I can debug this in Visual Studio?

LADSoft commented 3 years ago

i haven't had much luck with the OCC.CFG file in VS, so a command line I would use to debug with is:

occ /z\orangec\include;\orangec\include\next cgamma.c

where the current directory is set to

\orangec\src\clibs\cmath

... you might be able to compile the library with the fullbuild approach after this is fixed (but checkout my latest makefile fix first)

chuggafan commented 3 years ago
occparse Version 6.0.45.1 Copyright (C) LADSoft 2006-2020
Error( 11)    C:\OrangeC\include\__config(513):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(517):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(521):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(531):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(535):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(536):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(543):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(547):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(551):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(564):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(568):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(572):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(1251):  Syntax error: ) expected
Error( 11)    C:\OrangeC\include\__config(1458):  Syntax error: ) expected

I got the issue a few times, just running into this randomly for some reason, I'm also compiling with /IHello, for added complexity on my end.

LADSoft commented 3 years ago

yeah that was a recently introduced bug. It affected compiling C language files. Thought I had fixed it in one of the very latest pushes...

LADSoft commented 3 years ago

while the latest fix fixed cgamma.c c, now there is a problem with \orangec\src\clibs\alloc\rpmalloc.c. Same problem as far as I can tell, it includes stdint.h out of the libcxx headers which does an include_next into the c headers...

I can fix this and possibly other related problems that may be showing up by fixing #562 but it will rear its head again at some point in the future... when someone #includes stdint.h, complex.h , or one of several other similar headers in a C++ program...

chuggafan commented 3 years ago

I'm not seeing any problems, but I do see that for some reason we're emulating MSVC behavior for their atomics instead of.... using our behavior? But that's a separate issue....

Neither my naive just build it nor the omake fullbuild have any issues, are you sure you're running it with my changes?

LADSoft commented 3 years ago

I'll try again...

LADSoft commented 3 years ago

i'm not getting. I checked to make sure it was merged (it was) I checked to make sure I had the latest on the libcxx branch (I did) then I did a complete rebuild of the compiler since sometimes that is a problem. Didn't help. So I changed the name of the file in the include_next statement and made a new file to match, (had to do it for two separate headers to get rpmalloc to compile) and that fixed it. I'm seeing it on a variety of other files too though... can you check to make sure you checked in all the changes?

chuggafan commented 3 years ago

Cuz I'm going to go back to working on this, could you give me the parameter list that I can use to debug for the referenced issue? It would make development faster. If you can't I can go and find it even if it takes awhile, just will be slightly more annoying to get to.

LADSoft commented 3 years ago

compiling just about any C++ program which includes libcxx headers will show you, for example in visual studio do this:

occ /Z\orangec\include;\orangec\include\c \orangec\src\clibs\cpp\libextra\cregex.cpp