... let's try adding some other reviewers who've had their fingers in SemaOverload ...

Thank you for the review, Richard!

@rsmith I have no idea if the other test change is correct, and gnu cxxfilt doesn't work with that mangled name. Hope this doesn't actually break something...

Other than the declval issue discussed above, this looks reasonable to me. I don't notice anything wrong with the standard library reimplementations here.

I guess you need me or Michael to push this. Happy to do so once you're
happy with it.

I can't argue with this.

Looks great to me. Thanks, Art.

...in which we all remember why we like writing this recursively. :D

This is going to be so great. Thanks, Art.

Maybe add a comment on assertfail that we don't want it to be noreturn because blah blah? So someone doesn't come back and add it when we're not looking. :)

Thank you for the review!

I guess I was confused by the function name

Rename noteNumOverloadCandidatesShown -> overloadCandidatesShown.

Thank you for your comments, Aaron!

Not sure who can review this, but looking through blame it seems like maybe @aaronpuchert?

In D92210#2559113, @maleadt wrote:

Bump, anything else needed here?

Hi, welcome! Thank you for the careful and well-motivated first commit. (I also see https://github.com/ccache/ccache/issues/772, hooray for noticing that...)

This looks reasonable to me (and I appreciate all the debugging in the Julia bug!), but I have never touched this code, so I don't 100% feel comfortable approving the change.

In case it's not used in PHI or SELECT and cannot ensure the result is also a pointer to the parameter space, we could skip alloca insertion.

I don't believe there's any exception to prove deduction [of the readonly attribute] wrong.

This looks really simple, which is awesome. I am enthusiastic. But I am worried it may not be correct.

How fun. :)

I am legit excited about this if we could figure out how to make it work, but I don't have anything to add beyond what tra said.

The bottomline seems to be that the proposed formalism achieves the following: <snip>

LGTM. I think the change would make sense for CUDA, too. @jlebar - WDYT?

Man, phab doesn't make this easy, does it?

Hi. :) A few people pinged me asking for my feedback here, since I touched the convergent attr way back in the day, for CUDA.

LGTM modulo emankov's comment.

Fix bug in test command line, and fix commit message.

It does not quite match what we do here.

Hey, I'm leaving on a vacation tomorrow and didn't have a chance to get to
this review today.

It looks like 11.1 doesn't have a version.txt file

It should. I did mention in a previous comment that > Looks like the const-ness check should not be there, either. I need to revise the patch.

OK, now I'm starting to I understand this change..

OK, backing up, what are the semantics of static on __constant__, __device__, and __shared__?

wha... As you know, const doesn't mean anything, that can be const-casted away. And then you'll be able to observe that this nominally-static variable is just a normal variable.

I know it comes in a separate change, but can we add a check to the test-suite?

LGTM, and can we write a test in the test-suite?

tra and I talked offline and I...think this makes sense.

@RKSimon and others, thank you for the review and comments. Sorry for my delay here; this has changed from being my day job to my weekend hobby, and that made a bigger difference in my responsiveness than I'd like or than I expected.

Update per comments.

Thank you for the comments!

Submitted as a4384c756bd8a8190510. I'm working from my wife's machine and accidentally submitted with her name instead of mine. Oops. :)

Thank you for the reviews!

@RKSimon friendly ping

Do we have any nontrivially-long namespaces in LLVM?

In D74384#1872246, @thakis wrote:

Looks like this broke building on windows with clang-cl as host compiler in some situations: http://45.33.8.238/win/8160/step_4.txt

Thank you for the reviews!

Lint: Pre-merge checks

Rebase

Rebase

Rebase