- User Since
- Jul 12 2012, 2:19 PM (428 w, 2 d)
Fri, Sep 25
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.
Thu, Sep 24
Tue, Sep 22
Mon, Sep 21
Sun, Sep 20
Fri, Sep 18
Thu, Sep 17
Thanks, this is pretty subtle. Here's a small C++20 testcase that invalidates InsertPos while matching partial specializations:
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.
Wed, Sep 16
Mon, Sep 14
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.
Sun, Sep 13
@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
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.
Tue, Sep 8
Mon, Sep 7
- Address review comments.
- Add some positive tests.
Thu, Sep 3
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.
Wed, Sep 2
Tue, Sep 1
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.
Mon, Aug 31
Abandoned in favor of https://reviews.llvm.org/D82490
Thanks, I'm happy with this.
Fri, Aug 28
Aug 27 2020
Aug 26 2020
Aug 25 2020
Have you considered using the ext_vector_type attribute instead of vector_size for this? That would have a couple of advantages:
Aug 24 2020
Thanks, I'm happy with this approach.
Aug 22 2020
It would be useful to include in our documentation a brief list of which of these intrinsics can be used in constant expressions.
Aug 21 2020
(Same comments as for the bitreverse patch.)
Can you also add documentation to LanguageExtensions.rst? Thanks!
Aug 20 2020
Looks good to me. Do you need someone to land this for you?
Aug 19 2020
This looks OK to me.
Aug 18 2020
Here's a testcase that produces a name with external linkage: