Ping.

Committed. I updated the language in the fallible iterators section of the programmers manual too. Feed back on that is very welcome too.

Looks like this was LGTM'd but never applied. Stephen -- do you have commit access?

This was committed in r339809.

• Remove unused collection elements.

• Require iterator checks on each increment, rename some test vars.

• Rename test classes and remove fixture, add test for deref-returns-expected.

• Address several review feedback comments.

Assuming this looks good to everyone, I will also update the programmers manual documentation for fallible iterators. I wanted to get this into a final form before re-writing the docs though.

• Remove the cantFail added in r352888, since fallible_iterator makes it unneccessary.

• Fix a missing header, rename make_fallible_iter to make_fallible_itr, and update Archive::child_iterator to use the fallible_iterator wrapper.

Nice catch. LGTM. Thanks Zach!

LGTM! Do you have commit access?

This looks great. Thanks JF!

Hi Yonghong,

OK I think we've had more than enough bikeshed, over months, and in-person at the LLVM dev meeting. This is good enough, we want to warn people for the LLVM 8 branch which is tomorrow. Let's get this checked in today. We'll start a discussion on LLVM dev when anything *actually* changes, so nobody needs to panic just yet.

I strongly disagree with this right now: this is *not* OK to rush this as is.

If you need something in the release, then it can be done in a much less controversial way.

In D47073#1358039, @jfb wrote:

In D47073#1357505, @hans wrote:

Is there any particular reason why we'd need this for LLVM 8?

Yes: we want to warn folks early. This is just warning them, and they can opt-out. Getting it in the release branch is exactly the right place: people try out releases much more than tip-of-tree.

LGTM!

Gentle ping.

Apart from these specific comments, my general thought is that this is fine as far as it goes, but as it grows more fully featured it seems likely to start overlapping more libObject and MC functionality.

Committed in r348223.

Swinging back around to this, as I have a concrete use-case now (allowing JIT clients to query MaterializationUnit types when deciding how to dispatch compiler invocations in the JIT).

Updated to add documentation, comments.

Looks good to me. :)

Gentle ping.

Oops. Those comments were for and old diff. The no-op sorts are gone, but the other three still apply.

Nitpicks aside, the mach-o side of this looks good to me. :)

LGTM. Committed as r347281. Thanks Eugene!

Updates the PerfJITEventListener API and fixes a comment that referred to the old API.

In D53773#1280967, @anarazel wrote:

This looks like a sensible improvement. There's probably not many implementors of the interface, so the renaming shouldn't cause much pain.

I noticed however that you didn't update ExecutionEngine/PerfJITEvents/PerfJITEventListener.cpp?

• [ExecutionEngine] Rename/format JITEventListener methods.

Thanks everyone! Committed in r345411.

• Fix a missing &&

In D53726#1276405, @dblaikie wrote:

Patch description doesn't seem to immediately relate to the change - nor test cases demonstrating the connection?

• Add non templated constructors for DenseMap, clarify clang version in comment.
• clang-format

Oops -- forgot to clang-format this. Will do so before applying if it is accepted though.

Committed in r344995. Thanks again Andrew!

Hi Andrew,

Committed as r344542.

Use PowerOf2Ceil as per Craig's suggestion, and validate set contents in the non power-of-two length initializer list unit test as per Dave's suggestion.

Committed in r344522.

Updated to remove the dependence on stable iteration order for operator== (on both DenseMap and DenseSet). Also adds some comments on performance.

Broke out https://reviews.llvm.org/D53260 for the DenseSet bug fix.

I do not think this landed in the end (it does not look like it). It may be able to be rewritten more naturally now that LLJIT has native support for multiple threads. Also ThreadSafeModule and ThreadSafeContext will help with the context management issues.

I think the changes in r343122 provide a superset of this functionality, so this can be closed.