In D73007#1829709, @xbolva00 wrote:

Yes, but minimal fix is better for release branch, so @hans should merge it.

As I wrote on the bug, I think we should only suppress one of the warnings in templates (and maybe always):

Hmm, I have been wondering about this as well. The way I see it, all of these things are what we call capabilities, and we treat them all the same. The names are just meant to make warning messages more readable, because what the analysis considers a capability, the user might know as a mutex, or role, or sequence.

A fix would be to let all three produce exclusive negative capability, which essentially means there is no type associated with negative capability. This fix could also fix my bug.

@aaron.ballman We could make the warning even stricter, but that should be a separate discussion. Is this change Ok for now?

Ping.

In D65661#1615277, @morehouse wrote:

Without this patch, any project that wants to use this header needs to add a vendored copy of FDP to their source repository. With this patch, non-clang will still have to do this, though projects that build with clang will not.

In D70991#1778232, @ldionne wrote:

Is import distro supported in Python 2.x?

Already fixed via D66437.

An alternative would be to assume that any distribution new enough to have Python 3.8 also has /etc/os-release, so we fall back to parsing that if platform.linux_distribution isn't there. Or the other way around: we check for /etc/os-release and fall back to platform.linux_distribution.

@rsmith, could you please have a look?

In D69927#1750132, @JonChesterfield wrote:

The current change (without the racy test) looks good to me.

In D69927#1747168, @JonChesterfield wrote:

The function I was suggesting marking used is the one passed to the linker as -fini, not the one marked destructor.

Remove __kmp_internal_end_fini instead of exporting it: it shouldn't be needed.

In D69927#1745659, @JonChesterfield wrote:

There's probably an existing test for them.

In D69927#1744369, @AndreyChurbanov wrote:

foreign thread should crash if library closed w/o shutdown

I will try removing __kmp_internal_end_fini with a test that __kmp_internal_end_atexit is still called.

This is a larger effort so it will take a little bit more time on me.

Let's say we remove __kmp_internal_end_fini and rely on __kmp_internal_end_dtor, can we write a test case ensuring that __kmp_internal_end_atexit is called? That calls into __kmp_task_team_wait, so maybe we can exit while some tasks are still running, and get a different observable result depending on whether we wait or not?

In D69927#1737034, @AndreyChurbanov wrote:

As well as removing the __kmp_internal_end_fini symbol at all, as it duplicates the functionality of __kmp_internal_end_dtor which has attribute "destructor". I am OK with either solution, would be good to hear others opinion.

Belatedly filed bug 43927. Feel free to comment there if you think this isn't the proper fix.

@jrtc27, @nemanjai: Could you have a look again? I think I answered all your questions and wrote a test.

Gentle ping.

Not answering inline because comments are getting longer.

Add test case where check for non-deduced parameter conversions is necessary.

@beanz Could you have a look again?

Apply clang-format.

Given the complexities of this implementation, I'm beginning to doubt whether implicit moves make sense for co_return at all. Since there can never be any kind of RVO, why not always require an explicit std::move? Implicit moves on return were introduced because an explicit move would inhibit NRVO, and without move there wouldn't be a graceful fallback for implementations that don't have NRVO.

Add tests suggested by @Quuxplusone and add fallback to call by lvalue reference.

@GorNishanov Do I read the standard correctly that there are no constraints on what return_value is? That is, could it also be a function pointer or function object?

I'll add your test case, but I'll probably reuse the existing data structures.

In D51741#1702038, @Quuxplusone wrote:

Both of these should first do overload resolution for one parameter of type MoveOnly&&, and then, only if that overload resolution fails, should they fall back to overload resolution for one parameter of type MoveOnly&.

Also remove FIXME comment.

Please have a look at D68845. This should address the issues that we discussed.

@Quuxplusone Thanks for your very helpful comments!

In D51741#1702038, @Quuxplusone wrote:

This patch is heavily heavily merge-conflicted by P1825.

This change breaks the following code that worked before:

Gentle ping.

In D67112#1655937, @aaronpuchert wrote:

If anyone shares my feeling that the boolean output parameters of CompareReferenceRelationship should rather move to the return value, I would be happy to do that.

Ping?

Fixed by @aaron.ballman in rC373213 along with another issue.

Can be "reproduced" with clang -fsyntax-only -Wstrict-prototypes clang/include/clang-c/*.h.

Fix typo.

Handle static libraries correctly.

In D67877#1686961, @Jiboo wrote:

@Szelethus could you confirm that thus examples shouldn't be built when CLANG_BUILD_EXAMPLES is OFF, and that this patch is still valid?

In D68172#1686960, @Szelethus wrote:

The patch looks alright, I won't formally accept because if I knew how these worked, it wouldn't have caused so much pain to so many people :)

I didn't know about this patch and published D68172: adding BUILDTREE_ONLY still builds the examples, but doesn't install them. It's also used by other test plugins like llvm/lib/Transforms/Hello.
Thanks to @lebedev.ri for informing my about this patch.

In D68172#1686772, @NoQ wrote:

+@Szelethus because i'm a bit out-of-the-loop on plugins: i very vaguely remember that we decided to put them into tests(?)

In D55326#1342412, @nemanjai wrote:

A couple of questions since I am not all that familiar with clang and am certainly not familiar with this unusual SUSE 32-bit situation:

• We seem to be changing the set of aliases here, but what happens if someone actually explicitly specifies --target=powerpc-suse-linux?
• Can you describe the default triple for clang on SUSE 32-bit PPC? Will it be powerpc-suse-linux? powerpc64-suse-linux?

This change does not affect the Clang triple, that is and will be powerpc-unknown-linux-gnu. It's just about where Clang looks for the GCC toolchain.

Perhaps I should mention that this fixes an assertion failure.

Pass correct Clang triple as argument to --target.

I'm thinking about the following, but I'm not sure if that's the proper way to do it.

In D66919#1658355, @aaron.ballman wrote:

We do have numerous warnings that are default errors, you can look for DefaultError in the diagnostic .td files to see the uses.

Remove wrong changes in SemaExprCXX.cpp.

If anyone shares my feeling that the boolean output parameters of CompareReferenceRelationship should rather move to the return value, I would be happy to do that.

In D66919#1655048, @dexonsmith wrote:

I went back to read notes from when we deployed -Wstrict-prototypes (which we have had on-by-default for our users for a couple of years, since it catches lots of -fblocks-related bugs). I was mainly objecting because I had (mis)remembered trying and backing out the specific behaviour you're adding. On the contrary, our notes say that we might want to strengthen the diagnostic as you're doing.

Thank you for having a look at the notes, that's good to hear.

IIRC, when we rolled out -Wstrict-prototypes we explicitly excluded this case since it hit a lot of code without finding bugs.

We had a discussion on IRC yesterday and @aaron.ballman pointed out that the K&R syntax has been considered "obsolescent" (= deprecated) since C89, 30 years ago. He added that there are thoughts within the standards committee about removing the syntax entirely from C2x.

By the way, I'm open to adding a fix-it hint for zero-parameter K&R-style definitions, since the fix is pretty straightforward.

In D66919#1651108, @dexonsmith wrote:

we just don't warn on non-prototype defining declarations, where the meaning is unambiguous:

void foo() {}

"Meaning" is a difficult term. What we know is that this is a K&R-style definition, and does not define a prototype. So one would expect -Wstrict-prototypes to warn about this.

Note that I'm just copying GCC, which seems the be the original intent behind the warning (https://bugs.llvm.org/show_bug.cgi?id=20796). So people who use both compilers will have seen that warning already. Note also that there is no warning if any declaration provides a prototype, so this is fine:

Added a test case, verified that it fails before the change.

Sorry for warming this up again, but it would be nice to get rid of this patch in openSUSE.

In D41296#962215, @hfinkel wrote:

Why don't we just cap all names at some point (and then just start adding numbers as we generally do to break degeneracies). It seems like, otherwise, we'll end up with these kinds of fixes in many places. Fixing this in one common place seems better.