loarabia / Clang-tutorial

A collection of code samples showing usage of clang and llvm as a library
Other
754 stars 122 forks source link

Modifications made to CItutorial3.cpp to work with C files that include headers #16

Closed aseeda closed 12 years ago

aseeda commented 12 years ago

...orial3.cpp

loarabia commented 12 years ago

So I was looking at the change this morning and wondering why on some platforms the current code in tutorial 3 works but not on others. So I debugged down into the clang sources to check it out. What I noticed is that the code is getting into InitHeaderSearch.cpp in the InitHeaderSearch and AddDefaultIncludes but that the clang folks have removed the Linux (and Win32) handling and have pushed that code into the Driver:

switch (os) { 00258 case llvm::Triple::Linux: 00259 case llvm::Triple::Win32: 00260 llvm_unreachable("Include management is handled in the driver.");

That makes sense for the compiler as a whole but seems less good on first blush for libs and tools using clang.

@aseeda, I can accept your change but I'm wondering if there is a better way that the clang folks have in mind. Have you seen anything ?

aseeda commented 12 years ago

Hi Larry,

Bottom line on top: Not sure how to proceed because the code seems to be hard-coding the wrong include paths at least for Linux.

I took a quick look at what the Driver code is doing for Linux platforms (specifically llvm/tools/clang/lib/Driver/ToolChains.cpp) and it seems that they hard code where they expect the include libraries ought to be inside AddClangSystemIncludeArgs. It so happens that what they have for x86_64 does NOT match what I have setup on my machine so that explains why tutorial3.cpp didn't work for me.

The comment in InitHeaderSearch::AddDefaultIncludePaths seems to indicate that they are moving all the platforms into the Driver:

//================= void InitHeaderSearch::AddDefaultIncludePaths(const LangOptions &Lang, const llvm::Triple &triple, const HeaderSearchOptions &HSOpts) { // NB: This code path is going away. All of the logic is moving into the // driver which has the information necessary to do target-specific // selections of default include paths. Each target which moves there will be // exempted from this logic here until we can delete the entire pile of code. switch (triple.getOS()) { default: break; // Everything else continues to use this routine's logic.

case llvm::Triple::Linux: case llvm::Triple::Win32: return; } //============

So it seems to be they want to slowly migrate all the logic for all platforms to one place where (I hope) they would automatically detect paths.

Perhaps you can add in my changes at least temporarily to get others going while this include paths business gets sorted out.

What do you think?

Ammar

On Sun, Aug 26, 2012 at 11:50 AM, Larry Olson notifications@github.comwrote:

So I was looking at the change this morning and wondering why on some platforms the current code in tutorial 3 works but not on others. So I debugged down into the clang sources to check it out. What I noticed is that the code is getting into InitHeaderSearch.cpp in the InitHeaderSearch and AddDefaultIncludes but that the clang folks have removed the Linux (and Win32) handling and have pushed that code into the Driver:

switch (os) { 00258 case llvm::Triple::Linux: 00259 case llvm::Triple::Win32: 00260 llvm_unreachable("Include management is handled in the driver.");

That makes sense for the compiler as a whole but seems less good on first blush for libs and tools using clang.

@aseeda https://github.com/aseeda, I can accept your change but I'm wondering if there is a better way that the clang folks have in mind. Have you seen anything ?

— Reply to this email directly or view it on GitHubhttps://github.com/loarabia/Clang-tutorial/pull/16#issuecomment-8034215.

ofavre commented 12 years ago

I faced this exact issue in my project. I use the Driver API to let Clang use the appropriate Tool and ToolChain to add the required includes. I then intercept the commands to evict a process creation, but that's facultative. Here is the link to the interesting file: https://github.com/ofavre/TwistIDE/blob/c350250ca5d6d64977d9ed2656f4aeb132d9db96/tests/amnoid-clang-tutorial/src/inputargs.cpp

I'm not 200% happy of the solution, but still, it is a portable solution I think.

loarabia commented 12 years ago

Merging this in now sans makefile changes. Still looking for a long term solution that aligns with the internal changes that the LLVM / clang folks are up to.