Page MenuHomePhabricator

Introduce updateDiscriminator interface to DILocation to make it cleaner assigning discriminators.
ClosedPublic

Authored by danielcdh on Oct 25 2016, 12:52 PM.

Details

Summary

This patch introduces updateDiscriminator to DILocation so that it can be directly called by AddDiscriminator. It also makes it easier to update the discriminator later.

Event Timeline

danielcdh updated this revision to Diff 75771.Oct 25 2016, 12:52 PM
danielcdh retitled this revision from to Introduce updateDiscriminator interface to DILocation to make it cleaner assigning discriminators..
danielcdh updated this object.
danielcdh added a subscriber: llvm-commits.
aprantl added inline comments.Oct 25 2016, 1:21 PM
include/llvm/IR/DebugInfoMetadata.h
1274

"update" doesn't feel quite right. Would cloneWithDiscriminator() or something along those lines be any better?

1620

We usually use auto if the type is obvious from the context.
I think a while-loop may be less compact but more readable here? Or maybe just add a comment explaining what the loop does?

danielcdh updated this revision to Diff 75784.Oct 25 2016, 1:31 PM
danielcdh marked 2 inline comments as done.

update name and comment

aprantl accepted this revision.Oct 25 2016, 2:17 PM
aprantl edited edge metadata.

This really simplifies the code a great deal.

include/llvm/IR/DebugInfoMetadata.h
1274

Should we leave the inlining decision to the compiler?

1620

has -> have (2x)

This revision is now accepted and ready to land.Oct 25 2016, 2:17 PM
danielcdh updated this revision to Diff 75808.Oct 25 2016, 4:12 PM
danielcdh edited edge metadata.

Thanks for the quick review. Will hold this until tomorrow to see if other debug info maintainers have any comments about this API change.

echristo edited edge metadata.Oct 25 2016, 4:19 PM

I don't :)

-eric

danielcdh updated this revision to Diff 75901.Oct 26 2016, 8:57 AM
danielcdh edited edge metadata.

fix some typo

danielcdh closed this revision.Oct 26 2016, 8:58 AM