On Mar 22, 2013, at 2:52 PM, David Blaikie <[email protected]> wrote:
> Looks mostly reasonable:
>
> * if it has to be named "void" why does "createVoidType" take a string?
mostly for symmetry with createNullPtrType, which I copied ;-)
We can hardcode the name, but in theory there could be other languages that
have a void type with a different name.
> * why do you need the if void/DIType else/getOrCreateType? Why does
> the void type not fall out of getOrCreateType like any other type?
As the comment says, although a function without a return type is written as
void foo(), as far as DWARF is concerned, it has no return type (“null" in
llvm-ir), which is distinct from type "void".
> * you could underconstrain your Clang test case a bit further -
> probably drop the "null, null, metadata !"" from the second CHECK, I'm
> not sure it adds much (you don't really need to check that the first
> operand is i32 either: [[VOIDPTR]] = {{.*}}, metadata
> ![[VOID:[0-9]+]]} ; [ DW_TAG_pointer_type ]), remove the metadata
> checking in the 3rd line & just rely on the annotation comment (this
> makes the test cases more resilient to changes in the metadata format
> - it's easier for us to preserve the annotation comment correctness
> (at least that's been my feeling recently with all the schema changes
> I've been making), similarly with the CONSVOIDPTR line and CONST line
Agreed.
> * If you're updating the C++0x nullptr line anyway, perhaps you could
> s/C++0x/C++11/ while you're there (you could commit these nullptr
> changes separately if you like, so patches are more clear/specific)
Sure.
> * in DIBuilder.cpp would it be possible to quote the DWARF spec,
> rather than paraphrasing it? (see otehr examples of standards
> quotations in the codebase (there are many examples of C++11
> quotations in the Clang codebase)
The DWARF text is GNU FDL licensed. I was unsure if a verbatim citation might
create problems.
thanks,
Adrian
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
