- User Since
- Jul 12 2012, 2:19 PM (266 w, 4 d)
Fri, Aug 18
Thanks, will you need someone to commit this for you?
I'd like to also see a testcase for the situation where we trigger the emission of a declaration with an available_externally definition and then find we need to promote it to a "full" definition:
Thu, Aug 17
Oh, and you'll need an accompanying test case before this can be committed (look at the existing tests in test/SemaCXX for ideas).
Tue, Aug 15
Mon, Aug 14
Sun, Aug 13
Fri, Aug 11
This looks reasonable to me. Please consider whether we should take D36604 too.
Thu, Aug 10
Wed, Aug 9
Tue, Aug 8
Committed as r310401.
Mon, Aug 7
Remove added calls to DeclareImplicit* and ShouldDeleteSpecialMember. In their place, figure out whether an implicit special member would be deleted or not by querying the AST.
As requested by Vassil, I'm going to upload another version of this that avoids declaring implicit special members so frequently.
Sun, Aug 6
Sat, Aug 5
The current behaviour is correct according to the latest discussions of CWG. We shouldn't be delaying instantiation; we just need to fix the recent regression for functions where we try to instantiate functions that don't have a complete body yet.
Fri, Aug 4
FWIW, I see a 10% performance improvement on one of our internal benchmarks with this patch + force-precise-rotation-cost, compared to force-precise-rotation-cost alone.
Thu, Aug 3
This will need a test case.
Landed in r309975.
Sorry for the delay. Let's get this in for Clang 5.0. Thank you!
Wed, Aug 2
Organizationally, this seems fine. Carry on :)
Sun, Jul 30
Sat, Jul 29
Thu, Jul 27
Wed, Jul 26
Tue, Jul 25
LGTM with a tweaked comment. Thanks!
Mon, Jul 24
I would prefer to handle std::byte explicitly in the places where it matters, rather than adding an attribute to it and otherwise treating it like any other enumeration. We're going to want to be able to identify std::byte for warning emission and probably other codegen purposes too -- it's more a new fundamental type that happens to be defined in the library (like std::nullptr_t) than a type that just happens to be mayalias. Maybe a Type::is[Std]Byte, analogous to Type::isAlignValT?
I think we need more consideration of what the end result of __VA_OPT__ should be, and particularly how it should affect the SourceLocation data. It seems there are two models:
Jul 15 2017
Jul 9 2017
It looks like there are fewer special cases with this direction then our present one, though I worry that they'll be less obvious. On the whole, this seems like a improvement.
Looks like most of the users of this function are detecting whether we're in the builtins buffer. Perhaps we should have a dedicated method for that which uses something more principled than a string comparison against "<built-in>"?
Sure, it makes sense for UBSan to behave like other sanitizers in this regard.
This needs tests; perhaps extend test/Driver/linux-header-search.cpp for this.
Jul 6 2017
Jul 5 2017
We need to be completely clear on the differences and interactions between _Float16 and __fp16; you should include a documentation update that describes this as part of this change or as a companion change. In particular, an explanation of why we need two different types here is essential.
Some concrete suggestions throughout the patch, but I think we should take a step back and reconsider this warning approach: it seems bizarre for us to warn on any packed struct that happens to contain a char. It would make sense to warn if an __attribute__((packed)) written in the source has *no* effect (because it's applied to a struct where all fields already have alignment 1, or because it's applied to a field that has alignment 1) -- even then I'm not entirely convinced this is a valuable warning, but I assume there's some reason you want to warn on it :)
We've had problems in the past with speculative emission of values like this resulting in us producing invalid symbol references. (Two cases I remember: an available_externally symbol references a symbol that should be emitted as linkonce_odr but which is not emitted in this TU, and an available_externally symbol references a symbol with hidden visibility that is actually defined in a different DSO. In both cases, if the value of the available_externally symbol is actually used, you end up with a program with an invalid symbol reference.)
I don't especially like that we pretty-print the substituted subexpression to name the requirement, but I'm also not sure any other alternative would be better.
Jul 4 2017
Jun 30 2017
I'm not particularly happy with the number of different places where we duplicate the knowledge of how break/continue bind to enclosing loops, but fixing that seems beyond the scope of this change.
Thanks, do you need someone to commit this for you?
You should remove the code after the assert(0), or you'll trigger "unreachable code" warnings.
LGTM, post-commit review is fine for changes like this.