Skip to content

Conversation

@jchia
Copy link
Contributor

@jchia jchia commented Nov 26, 2016

This commit adds new Type functions in Language.C.Clang.Type. They allow obtaining complete layout information of more complicated structs than before the commit.

I'm not sure what naming convention you prefer for the new functions, but I decided to prefix them with 'type' to make it clear that they return something about a given type. If you have other thoughts, please let me know.


This change is Reviewable

@jchia
Copy link
Contributor Author

jchia commented Nov 26, 2016

Additionally, I'm not sure what's up with the existing Travis failures or what I should do about them.

@chpatrick
Copy link
Owner

Review status: 0 of 3 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


src/Language/C/Clang/Internal/FFI.hsc, line 557 at r1 (raw file):

  res <- [C.exp| long long { clang_Type_getSizeOf(*$(CXType *tp)) } |]
  return $ case res of
    #{const CXTypeLayoutError_Invalid} -> Left TypeLayoutErrorInvalid

Let's factor out this error checking and share it with offsetOfField.


Comments from Reviewable

@jchia
Copy link
Contributor Author

jchia commented Nov 27, 2016

How do we manage the commits? Multiple commits or one squashed commit? I don't know whether github will recognize my intention if I squash multiple commits into one and then force-push.

@chpatrick
Copy link
Owner

I'm getting this on Travis:

src/Language/C/Clang/Internal/FFI.c:184:1: error:
     error: implicit declaration of function ‘clang_Cursor_getOffsetOfField’ [-Werror=implicit-function-declaration]
     return ( clang_Cursor_getOffsetOfField(*cp_inline_c_0) );

I guess this was only added in a recent version of libclang? Do you happen to know which one?

@jchia
Copy link
Contributor Author

jchia commented Nov 27, 2016

I don't know, but I think it was added no later than 3.8.0 as I can build chpatrick/clang-pure master fine using clang 3.8.0.

@jchia
Copy link
Contributor Author

jchia commented Nov 27, 2016

Comparing these two files for occurrence of the function name, it seems that the first appearance is in 3.7.
https://github.com/llvm-mirror/clang/blob/release_36/include/clang-c/Index.h
https://github.com/llvm-mirror/clang/blob/release_37/include/clang-c/Index.h

@chpatrick
Copy link
Owner

Merged, thanks!

@chpatrick chpatrick closed this Nov 28, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants