This is an archive of the discontinued LLVM Phabricator instance.

Expose the Clang::QualType to llvm::Type conversion functions
ClosedPublic

Authored by Praetonus on Jul 9 2017, 6:24 AM.

Details

Reviewers
rjmccall
Summary

This change exposes the various CodeGenTypes::ConvertType functions in the public Clang API. This allows external tools using the Clang API to rely on it to generate LLVM types from C types. The precise use case that motivated this change is explained in an email to the Clang mailing list.

Diff Detail

Event Timeline

Praetonus created this revision.Jul 9 2017, 6:24 AM
rjmccall edited edge metadata.Jul 9 2017, 11:26 AM

Exposing these operations seems fine, but I'm not thrilled about the APIs. Of course, the APIs are exactly derived from IRGen's internal APIs, but we'd like to eventually clean those APIs up, and we certainly don't need to emulate them.

include/clang/CodeGen/CodeGenABITypes.h
74

Is this one actually important to expose? What sorts of future APIs are you expecting where clients will interact with IRGen other than in memory?

If we do need this, we should name it something clear so that people don't accidentally reach for it by default. (This has actually been a pain point within IRGen for years.) How about:

llvm;:Type *convertTypeForRValue(CodeGenModule &CGM, QualType type);

78

Let's make this a specific API for getting the type of a function declaration.

llvm::FunctionType *convertFreeFunctionType(CodeGenModule &CGM, const FunctionDecl *FD);

80

llvm::Type *convertTypeForMemory(CodeGenModule &CGM, QualType type);

Praetonus updated this revision to Diff 105793.Jul 9 2017, 1:26 PM

Patch updated based on @rjmccall's comments. I removed the wrapper for the ConvertType function, after giving it some thoughts I don't see any use case for it, I guess I added it by default. I left the convertFreeFunction function as returning an llvm::Type instead of an llvm::FunctionType as ConvertFunctionType represents incomplete function types as empty aggregate types.

Hmm. Maybe it would make more sense to allow it to return null, and then add a comment explaining that it will do so if it can't lower the function type yet.

Praetonus updated this revision to Diff 105795.Jul 9 2017, 3:59 PM
Praetonus marked 3 inline comments as done.

Patch updated. I've made the convertFreeFunctionType return null on failure.

rjmccall accepted this revision.Jul 9 2017, 4:37 PM

Looks great, thanks!

This revision is now accepted and ready to land.Jul 9 2017, 4:37 PM
majnemer added inline comments.
lib/CodeGen/CodeGenABITypes.cpp
71

Pointers lean right.

73

need a space between the if and the parenthesis.

Praetonus updated this revision to Diff 105803.Jul 9 2017, 7:51 PM

Patch updated to adhere to the coding style based on @majnemer's comments.

rjmccall accepted this revision.Jul 11 2017, 10:28 AM

Still looks good to me. Do you need help committing?

I forgot to mention that I don't have commit access. If you or somebody else could commit this it would be great.

rjmccall closed this revision.Jul 12 2017, 12:45 AM

Committed.