DIFactory

Project Infrastructure LLVM Dev List Archives
1

Seems the last use of DIFactory in LLVM/Clang is in:

clang/lib/CodeGen/CGDebugInfo.cpp to get the enums
llvm::DIFactory::OpDeref and llvm::DIFactory::OpPlus.

Shouldn't this be moved to DIBuilder and remove the dependency completely?

2

I didn’t know DIFactory existed until you mentioned it just now.

And if folks are adding brand new classes to LLVM, can we not follow the naming conventions in the developer guidelines?

3

Sorry, I meant DIBuilder.

4

DIBuilder is the new DIFactory. I've been playing with it this week
and it's much easier and straightforward to use. I'm still having
problems to create arrays, though.

As far as I remember (from the 2010 meeting), the idea was to replace
and deprecate DIFactory.

I'm not saying we should do it now, just saying Clang should have no
more deps on the old DIFactory to avoid header pollution, since it's
only on one enum... :wink:

cheers,
--renato

5

Sorry, I meant DIBuilder.

DIBuilder is the new DIFactory. I’ve been playing with it this week
and it’s much easier and straightforward to use. I’m still having
problems to create arrays, though.

As far as I remember (from the 2010 meeting), the idea was to replace
and deprecate DIFactory.

I’m not saying we should do it now, just saying Clang should have no
more deps on the old DIFactory to avoid header pollution, since it’s
only on one enum… :wink:

Yeah, I remember that conversation - I just didn’t know that any action had been taken.

However, my other concern is this: According to the LLVM Coding Standards document (http://llvm.org/docs/CodingStandards.html#ll_naming):

Function names
should be verb phrases (as they represent actions), and command-like function should be imperative. The name should be camel case, and start with a lower case letter (e.g. openFile() or isFoo()).

The coding standards say functions should begin with a lower case letter, but I see a lot of new code (not just DIBuilder) that uses method names that begin with an upper case letter. Is the document incorrect, or is there basically no enforcement?

6

A couple of other questions about DIBuilder:

  1. I notice that there’s no ‘isMain’ parameter to CreateCompileUnit. How do you specify the main module?

  2. There’s no means to set TheCU other than creating a new compile unit. This means that you have to generate all of the debug info for your module with a single DIBuilder instance, since there’s no way to use a pre-existing compile unit.

7

DIBuilder is older than the naming standards. Patches to improve DIBuilder’s naming would definitely be appreciated.

-Chris

8

Sorry, I meant DIBuilder.

DIBuilder is the new DIFactory. I’ve been playing with it this week
and it’s much easier and straightforward to use. I’m still having
problems to create arrays, though.

OK I switched all my stuff over to use DIBuilder, and you are right - the arrays don’t work. Everything else does though.

9

Phew! I thought I was just being Friday-stupid... :wink:

There is a very thin line that could make it work (the conversion to
MDNode* from operator* and then automatically become a Value*), but I
don't think that would work and even if it did, it's bad practice
because it's a completely unrelated feature.

cheers,
--renato

10

A few more concerns:

By putting the copy ctor in private, I can't define a DIBuilder in a
context and assign it later with a respective module (or re-assign it
for that matter).

To overcome this, I've created a pointer to a DIBuilder and only
create it when I have the module information. I have to do this
because the front-end is not in C++, so I can't use the ctor init
trick on DIBuilder like clang does.

But that seems to be getting in the way of the DIDescriptor "operator
MDNode *" or whatever is converting a DIDescriptor into a Value* in
the code below (CGDebugInfo.cpp):

  llvm::Value *Subscript = DBuilder.GetOrCreateSubrange(0, NumElems);

since GetOrCreateSubrange() returns a DISubrange (not a pointer) both
in DIFactory and DIBuilder.

Any pointers? (pardon the pun)

11

TheCU is an internal debug info information that FE should not care about. DIBuilder is meant to use for one translation unit by FE. If all the internal debug info information is exposed to FE then you'll get DIFactory.

12

I agree, DIBuilder should not expose its internal structure. This is
why, on a C-only world, it's essential that you separate declaration
from instantiation on DIBuilder.

So far, it's been easy to do that with DIFactory (using a pointer to
it), but DIBuilder's get-range makes it difficult. Can you change it
to accept a list of DIDescriptors rather than Value*? I think for the
metadata it doesn't make any difference and in the back-end you can
always convert it.

cheers,
--renato

13

If you have patch, go ahead, I am fine with it.

14

I don't, but can produce one. Will look into it tomorrow morning.

cheers,
--renato

15

Hi Devang,

After meddling with CGDebugInfo, I now know why GetOrCreateArray
accepts a list of Value** instead of DIDescriptors: arrays are used
for many other things than just types.

Talin,

To make arrays work you just need to include "Metadata.h" in your
file. When forcing a conversion through MDNode* -> Value*, I finally
got a meaningful error "MDNode type is incomplete", that led me to
including Metadata and getting the behaviour CGDebugInfo has when
putting DISubrange into a Value* vector.

cheers,
--renato