CIndex changes

Ted/Steve,

Do you want me to check in my changes for DLL-izing CIndex?

I’d like to do so, because Index.h always conflicts when I update.

Here’s a patch against the latest, with just the changes for getting the DLL to build.

It would be nice if you could fix the string problem too, so the test doesn’t fail on Windows.

-John

cindex.patch (8.07 KB)

Ted/Steve,

Do you want me to check in my changes for DLL-izing CIndex?

Please go ahead, pending the comments below. Thanks!

I'd like to do so, because Index.h always conflicts when I update.
Here's a patch against the latest, with just the changes for getting the DLL to build.

In this:

+// MSVC DLL import/export.
+#ifdef _MSC_VER
+ #ifdef _CINDEX_LIB_
+ #define CINDEX_LINKAGE __declspec(dllexport)
+ #else
+ #define CINDEX_LINKAGE __declspec(dllimport)
+ #endif
+#endif

we need a

#else
# define CINDEX_LINKAGE

for non-Microsoft compilers.

-typedef void (*CXTranslationUnitIterator)(CXTranslationUnit, CXCursor,
+CINDEX_LINKAGE typedef void (*CXTranslationUnitIterator)(CXTranslationUnit, CXCursor,
                                            CXClientData);

We don't need dllimport/dllexport on typedefs, right?

  - Doug

Thanks, Doug. It’s in 85234.

we need a

#else

define CINDEX_LINKAGE

for non-Microsoft compilers.

Yep, sorry, I missed that. It’s in now.

We don’t need dllimport/dllexport on typedefs, right?

Right. I fixed this too.

How about the string problems I ran into? Is there a consensus yet on what to do about them?

-John

Thanks, Ted.

I’m sorry I’ve been disconnected from this. What particular code were you looking at that returned a ‘const char*’ that referred to the buffer associated with a temporary std::string?

It’s in clang_getTranslationUnitSpelling, clang_getDeclSpelling, and clang_getCursorSpelling. The enclosed patch with my hacky work-around will show the specific places.

I also moved getLocationFromCursor to fix a VC++ message about the return type not being permitted inside an extern “C” {} block.

-John

cindex_strings.patch (7.48 KB)

Thanks John. The patch makes it very clear.

The problem is that we don’t want to add static data to CIndex, as we want the functions to be as reentrant as possible.

Steve/Doug: I believe you guys discussed this one. Comments?

Thanks John. The patch makes it very clear.

The problem is that we don’t want to add static data to CIndex, as we want the functions to be as reentrant as possible.

Steve/Doug: I believe you guys discussed this one. Comments?

Hey Ted,

We haven’t discussed it yet. I expect to speak with Doug today/tomorrow and make a decision by the end of the week.

snaroff

Hi,

What’s the status of this issue? (CIndex string returns clobbered)

From purely a programming point of view, I still think changing the API for these functions to have the user pass in a string buffer and max length and then return the buffer pointer seems the most reasonable thing to do. But I don’t know the extent of your client impact, which I presume is the reason for the reluctance to change the API

-John

Hi,

What’s the status of this issue? (CIndex string returns clobbered)

From purely a programming point of view, I still think changing the API for these functions to have the user pass in a string buffer and max length and then return the buffer pointer seems the most reasonable thing to do. But I don’t know the extent of your client impact, which I presume is the reason for the reluctance to change the API

Hi John,

Since these are new API’s, the client impact isn’t that big a concern.

There is no doubt that having the client allocate the buffer is the simplest solution. My concern is making the API inconvenient.

I will make a decision on this within the hour:-)

Thanks for the follow-up…

snaroff