In D140074#4159583, @serge-sans-paille wrote:

The cleanup looks good to me, thanks! Out of curiosity, did just stumble on this, or did an automatic tool gave you an hint?

rebase

rebase

Fix test

(As in my previous commit, picking based on git log, please review / forward as appropriate. Thanks again!)

Unclear to me who to request review from, so I'm peeking reviewer based on the git log. Please feel free to review or forward to someone else as appropriate. Thanks in advance!

Remove gratuitious whitespace changes

In D122963#3457431, @rnk wrote:

gcc: both caller and callee do the zero/sign extension.

The way I read that, GCC and Clang are ABI compatible with each other.

In D71178#3424386, @lebedev.ri wrote:

This seems backwards.
Who added those signext/zeroext attrs to the function arguments?
Presumably they just shouldn't be added if that is not what ABI states.

Could anyone update me with how do they want me to proceed here? Is fixing the coercions enough to allow this to land? Do I need to make it target-specific? If so, which targets should keep the current behavior?

In D72742#1822554, @rnk wrote:

+@chandlerc @rjmccall

Didn't we work this out already when John added the alignment tracking stuff? I remember this bug involving libjpegturbo standalone assembly receiving a 32-bit argument, and then using the full 64-bit RDI register to read it, but clang stopped zero extending it. I thought the ABI doc was ambiguous at the time, and we studied GCC's behavior, and that is how we arrived at clang's current behavior. I am generally skeptical about changing behavior in this area just because the ABI doc says something now. It keeps changing. What did it say in the past? Shouldn't we pay more attention to that?

Found the libjpegturbo thing:
https://github.com/libjpeg-turbo/libjpeg-turbo/commit/498d9bc92fcf39124b6f08e57326944dedd2ddd6

The relevant discussion is here. From the "Parameter Passing" section in https://github.com/hjl-tools/x86-psABI/wiki/x86-64-psABI-1.0.pdf, there's no mention of sign-extension requirements for arguments.

So I started doing that, and D72742 looked different enough that I thought I would submit it separately. Do you think that's on the right track? Or have I missed something? Thanks.

In D71178#1774433, @craig.topper wrote:

Shouldn’t we instead stop the sext and zext attributes from being added by the frontend?

update tests

Oh, and thanks for the utils/update_llc_test_checks.py tip, I was updating them manually and it was driving me crazy :)

Could I get some feedback on this? Does this look reasonable?

Thanks for the review as always @Anastasia ^.^

Thank you for the review! :)

Thank you for all the reviews! :)

Huh, somehow forgot to press "Submit" this morning :)

Landed with that change, thanks for the review @Anastasia!

Add CHECK tests.

I sent http://lists.llvm.org/pipermail/cfe-dev/2017-May/053703.html

I'm personally fine with this behavior in particular, given you can check the cursor you're querying to know if it's a typedef or alias specialization. Of course I'm not the only one using libclang, though, so I'd be interested in hearing other people's opinion.

FWIW when I wrote D26663, I did it because arguments weren't inspectionable at all for using declarations. Actually both of them would've worked for me.

Revert + tests @ https://reviews.llvm.org/D32566

Updated per review comments, thanks :)

In D32348#738704, @rsmith wrote:

This change looks like it introduces a regression itself: given

template<typename T> struct A {};
template<typename T> using B = A<T*>;
B<int> bi;

... requesting the template arguments for the type B<int> changes from producing int to producing int* with this patch, which seems to directly oppose the intentions of D26663.

Updated per comments, I used clang_TargetInfo_dispose following recent APIs instead of clang_disposeTargetInfo, let me know if I should change that.

Err, that comment was supposed to be a reply to https://reviews.llvm.org/D32348#737058, oh well, thanks again :)

Thanks a lot for the tip! :)

Sounds good to me, will do :)

Yes, please! I've submitted a few patches, but still no commit access.

I'd appreciate if anyone could point me to an appropriate reviewer for this.

Full diff, thanks @arphaman :)

Closing per advice in cfe-commits@.

Ping?

Guy, perhaps you could also review this? (Just from looking at the blame log).

@skalinichev ping? Anything else I need to do to get this landed?

Now without a really dumb off-by-one, hidden by the fact that we always indexed in-range and packs are only the last element.

Actually it was not so bad, thanks @skalinichev :)

Thanks for the review, good catches :)

Updated per comments. Not super happy with making these functions linear, but given template arguments aren't that numerous usually, it's probably acceptable.

Updated per comments, let me know if I should do anything else.

Thanks for the comments!

@skalinichev Yeah, I noticed that and published https://reviews.llvm.org/D26907 a while ago, it hasn't been accepted yet though.

Sorry @skalinichev, didn't know about those. Thanks for fixing.

Now with proper tests :)

Hmm... You're right.

(Also, please don't rush if I'm wrong being concerned about releases or similar, I'm fairly ignorant in that regard)

Hi, @EricWF or @akyrtzi, could I get this reviewed?

Updated to add a missing null check, and remove unnecessary diff noise.