I think we established rough consensus in CWG (at least, no one objected) that this is a grammar bug -- there's nothing for these attributes to appertain to, so they shouldn't be permitted. Unless CWG indicates it wants to change direction here I think we should continue to reject.

In D87808#2288186, @dblaikie wrote:

In D87808#2286664, @rsmith wrote:

But I think this code should give the same outcome either way, because a class with any constructor other than a default/copy/move constructor must have a user-declared constructor of some kind, and so will never have an implicit default constructor.

Hmm, trying to parse this. So if we're in the subtle case of having an implicit default constructor (and no other (necessarily user-declared, as you say) constructors) - if it's not been declared, then this function will return false. If it has been declared, it'll return false... hmm, nope, then it'll return true.

In D87808#2280197, @dblaikie wrote:

@rsmith What's the deal with these anonymous structs/unions? Why do they have copy/move constructors (are those technically called from the enclosing class's copy/move constructors?) but no default constructor to be called from the other ctors of the enclosing class?

Thanks!

Thanks, this is pretty subtle. Here's a small C++20 testcase that invalidates InsertPos while matching partial specializations:

In D77491#2280096, @jrtc27 wrote:

If someone cares about pthread_create they might wish to follow up on my https://reviews.llvm.org/D58531, which I filed early last year to permit pthread_create to have a proper type in the syntax. It will likely need rebasing, but probably isn't that much work.

The mechanism by which this interacts with Clang looks good to me. I've not done any detailed analysis to check all the functions made constexpr by P0784 are handled by this patch, though.

Thanks! This looks good to me (subject to Aaron's comment being addressed). Please wait a couple of days for any more comments from the other reviewers.

In D87528#2270561, @rjmccall wrote:

Richard, what do you think the right rule is for when to use the special constant-evaluation rules for this feature in C++? The C standard says that constant expressions should use the default rules rather than considering the dynamic environment, but C can get away with that because constant expressions are restricted to a narrow set of syntactic situations.

@rsmith asked "I don't see changes to the constant evaluator". The semantic rules for enabling this pragma require that strict or precise semantics be in effect., see SemaAttr.cpp And the floating point constant evaluation doesn't occur when strict or precise

In D86048#2254607, @sammccall wrote:

The crux is we're forcing decltype(N) to be a (unique) dependent type, which feels wrong.
[...]
There are three behaviors:

• standard up to C++14: traversing the expr looking for a template param to be lexically included (this is my reading of the standard)

For what it's worth, the most recent thoughts from CWG on this question are from 2015 (prior to guaranteed copy elision, which probably changes the answer): http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1965. That direction would permit const_cast to rvalue reference type to materialize a temporary for arrays and classes but not scalar types (which seems just as arbitrary as the current set of rules to me).

I made a start on this a while back and found a bunch more places that needed updates: https://github.com/zygoloid/llvm-project/commit/c23440979ac4f07ce38657ce0e69d010d2afa83e

Looks like this is an implementation of DR1307. Please also add a test to test/CXX/drs/dr13xx.cpp.

• Address review comments.
• Add some positive tests.

It seems to me that adding new __builtin_* functions for GCC compatibility and adding new LIBBUILTINs are unrelated changes, and it might be clearer to split them up into two commits.

In D86993#2251198, @rjmccall wrote:

Wording looks good. Should we alsso document our assumptions about what functions exist in the standard library — the functions that we'll always use even in freestanding builds?

In D85778#2251160, @zequanwu wrote:

Hi, this change seems like hits a false positive case in chromium build: https://bugs.chromium.org/p/chromium/issues/detail?id=1124085

In D85778#2248714, @nikic wrote:

Just for the record, the additional analysis has a measurable compile-time impact (0.3% at O0): https://llvm-compile-time-tracker.com/compare.php?from=e7f53044e7263cdbbb4fed9abf086b88ba486bba&to=cff6dda604cb0548bef5e5951dd1e74536110588&stat=instructions

Add missing word.

Documentation patch for your consideration: https://reviews.llvm.org/D86993

I wonder if there's a cleaner way to model this:

Looking specifically for attributes in the 'then' and 'else' cases of an if seems like a fine first pass at this, but I think this is the wrong way to lower these attributes in the longer term: we should have a uniform treatment of them that looks for the most recent prior branch and annotates it with weights. We could do that either by generating LLVM intrinsic calls that an LLVM pass lowers, or by having the frontend look backwards for the most recent branch and annotate it. I suppose it's instructive to consider a case such as:

Sorry for not noticing this review earlier.

I've not checked all the types are correct (someone should double-check that prior to commit), but it looks like GCC provides these __builtin_* functions, so we should too. The addition of the non-__builtin_ versions should improve our diagnostics but otherwise have no effect.

Thanks!

Abandoned in favor of https://reviews.llvm.org/D82490

In D77491#2246948, @rjmccall wrote:

I'd still like @rsmith to sign off here as code owner.

Thanks, I'm happy with this.

In D86815#2245467, @arsenm wrote:

In D86815#2245466, @arsenm wrote:

Why would we change this? What's the point of having separate memcpy and memmove intrinsics?

I'm reading this as clang mis-uses llvm.memcpy when it probably should be using llvm.memmove

Have you considered using the ext_vector_type attribute instead of vector_size for this? That would have a couple of advantages:

Thanks, I'm happy with this approach.

It would be useful to include in our documentation a brief list of which of these intrinsics can be used in constant expressions.

(Same comments as for the bitreverse patch.)

Can you also add documentation to LanguageExtensions.rst? Thanks!

Looks good to me. Do you need someone to land this for you?

This looks OK to me.

Here's a testcase that produces a name with external linkage: