[patch][PR10367] Fix the design of GlobalAlias v2

Duncan P. N. Exon Smith dexonsmith at apple.com
Thu May 15 19:48:08 PDT 2014


On 2014-May-14, at 13:00, Reid Kleckner <rnk at google.com> wrote:

> Looks good, but I'd let Duncan comment.

LGTM too.

> There is an instance of "Aliassee" in clang-3.patch that should probably be made consistent with the rest.
> 

At the top of clang-2.patch, you've changed `Aliasee` to `Aliassee` in a
couple of places.  I think the old name is better.  (Or is this what Reid
was pointing out?)

In llvm-2.patch in `LLParser::ParseAlias()`, you have a variable called
`AliaseType`, which is strange.  `AliaseeType`?  `AliasType`?

> 
> On Wed, May 14, 2014 at 12:08 PM, Rafael EspĂ­ndola <rafael.espindola at gmail.com> wrote:
> Starting a new thread to get a fresh view of the state of the patches.
> 
> The introduction of GlobalObject was already committed. These patches are
> * rebased on top of trunk.
> * include the feedback from the previous code review. Let me know if I
> missed anything.
> * Splits the manual removal of getAliasedGlobal to a separate patch.
> * Includes some linker fixes I found while rereading the previous version.
> 
> One thing that I noticed with the discussion about the C api is that
> LLVMAddAlias should probably not be changed, even if the constructor
> of GlobalAlias is changed. I will try to split this into an
> independent patch.
> 
> Cheers,
> Rafael
> 





More information about the llvm-commits mailing list