codegening constants

Hi,

I've noticed that currently clang's codegen isn't marking some consts as llvm constants (e.g. const char = "foo").
The attached patch fixes just that. As it adds some methods to QualType and Type classes, I'm sending to the list to get approval before commit. So please comment on it! :slight_smile:

Thanks,
Nuno

clang_emit_const.txt (3.2 KB)

The getAsArrayType* methods were moved to ASTContext for a reason...
you shouldn't be moving them back.

-Eli

Yep, this change should not pass the regression test suite. Nuno, did you test it?

-Chris

Yes, and it does pass the whole regression test suite (minus one test that was already failing).
Also, I didn't see any method in ASTContext that is appropriate to what I needed. getAsArrayType() in ASTContext are (in my understanding) to make new types, not to cast a Type* into an xxArrayType*.

Nuno

I've noticed that currently clang's codegen isn't marking some consts as
llvm constants (e.g. const char = "foo").
The attached patch fixes just that. As it adds some methods to QualType and
Type classes, I'm sending to the list to get approval before commit. So
please comment on it! :slight_smile:

The getAsArrayType* methods were moved to ASTContext for a reason...
you shouldn't be moving them back.

Yep, this change should not pass the regression test suite. Nuno, did you test it?

Yes, and it does pass the whole regression test suite (minus one test that was already failing).

Ok

Also, I didn't see any method in ASTContext that is appropriate to what I needed. getAsArrayType() in ASTContext are (in my understanding) to make new types, not to cast a Type* into an xxArrayType*.

getAsArrayType can return a different type than passed in. Specifically, if you ask for the array type of a "const array of int", you will get back an "array of const int" instead of an "array of int".

-Chris

I've noticed that currently clang's codegen isn't marking some consts as
llvm constants (e.g. const char = "foo").
The attached patch fixes just that. As it adds some methods to QualType and
Type classes, I'm sending to the list to get approval before commit. So
please comment on it! :slight_smile:

The getAsArrayType* methods were moved to ASTContext for a reason...
you shouldn't be moving them back.

Yep, this change should not pass the regression test suite. Nuno, did you test it?

Yes, and it does pass the whole regression test suite (minus one test that was already failing).

Ok

Also, I didn't see any method in ASTContext that is appropriate to what I needed. getAsArrayType() in ASTContext are (in my understanding) to make new types, not to cast a Type* into an xxArrayType*.

getAsArrayType can return a different type than passed in. Specifically, if you ask for the array type of a "const array of int", you will get back an "array of const int" instead of an "array of int".

From my quick testing I think I was getting 'correct' results. Anyway I

include a second patch in attach that uses the methods in ASTContext. (I dunno why I missed those on the first time..).
What do you think about this second version?

Nuno

clang_emit_const2.txt (1.62 KB)

This patch looks great to me. Eli, is this sufficient to handle the cases you know of?

-Chris

It looks correct to me; I can't think of any cases where this isn't
the right thing.

-Eli

Me neither, please apply Nuno, thanks!

-Chris

commited!
thanks for reviewing,
Nuno