- User Since
- Jul 12 2012, 2:19 PM (258 w, 3 d)
Fri, Jun 23
Unlike with sized deallocation (which also requires a new stdlib entry point), it was intentional that this be enabled by default in C++17 mode. Implicit calls are only generated to the aligned forms of operator new and operator delete in cases where the program would otherwise be broken (due to requiring more alignment than the allocator would provide). A link error seems strictly preferable to silent misalignment at runtime. Plus, you have no way to tell whether the user is providing their own aligned allocation / deallocation functions, so disabling this by fiat for certain targets breaks legitimate use cases -- we need to at least respect the command-line -faligned-allocation flag and only use this target-based detection as a default / fallback.
Thu, Jun 22
The remaining missing pieces were ObjCInterfaceDecl::visible_categories and ObjCInterfaceDecl::visible_extensions. The new revision of the patch gets a bit further down the road of passing through real visibility information into those, but that approach seems to snowball out of control. For example, ASTContext::mergeTypes now needs visibility information in order to determine whether two ObjC object types are compatible (because this depends on whether the types have compatible sets of protocols, which in turn can vary depending on which categories are visible for those two types). I have not completed this patch: there are still a lot of places that an IsHiddenFunction would need to be plumbed through to make this work.
Wed, Jun 21
It'd be good to also add tests for the other traits you're changing.
Tue, Jun 20
Looks good, thanks!
Mon, Jun 19
We use the -Wc++NN-compat- prefix on all the other subwarnings of -Wc++NN-compat warnings (examples: -Wc++98-compat-bind-to-temporary-copy, -Wc++11-compat-reserved-user-defined-literal, ...). I'd prefer to include the -compat in the name for consistency; I also find it less clear what the warning flag would mean without it (I would expect -Wc++1z-mangling to only fire in C++1z mode, whereas this warning never fires in C++1z mode).
Fri, Jun 16
If the lookup table is expected to be computed from the lexical decls rather than provided via "external visible storage", this makes sense to me.
Thu, Jun 15
Tue, Jun 13
Mon, Jun 12
LGTM as-is, but a couple of questions for possible further improvements.
Generally, all behavioural changes should have accompanying tests. The code change itself looks good, thanks.
Sun, Jun 11
Fri, Jun 9
This is a libstdc++ bug: https://gcc.gnu.org/PR79162 (They implemented a speculative fix for a different issue and ended up breaking this case.)
Looks great, thanks!
Is there any way to add a test for this? Does it affect the behavior of clang or any in-tree clang-based tools?
Thu, Jun 8
Wed, Jun 7
Tue, Jun 6
If you want to take this path, you should also add ActOnStartOfFunctionBody calls to the other parts of the parser that parse function definitions: Parser::ParseLateTemplatedFuncDef, Parser::ParseLexedObjCMethodDefs, and Parser::ParseLexedMethodDefs. You should also rename ActOnFinishFunctionBody to something like ActOnFinishFunctionDef to avoid confusion, because it pairs with ActOnStartOfFunctionDef not the new ActOnStartFunctionBody.
Looks good to me, thanks! Do you need someone to commit this for you?
Mon, Jun 5
Looks good, with a couple of tweaks (and corresponding test changes).
Fri, Jun 2
Thu, Jun 1
Rebased and added requested test.
Wed, May 31
My feeling is that there's not much value in adding an LLVM intrinsic for something that can be expressed directly in a handful of IR instructions; those instructions seem like the way to express this that would allow the most optimization potential, and the backend should be able to pattern match this to a BT instruction. Based on that, I'm reluctant to add clang builtins for this, partly because it looks like we'd need quite a lot of them and partly because (in the absence of an IR intrinsic) they'd just get lowered into the same instructions as the plain C code anyway.
Tue, May 30
Mon, May 29
Sun, May 28
Sat, May 27
May 26 2017
May 25 2017
Do we need to conditionalize this part of libc++? Nothing in the <coroutines> header appears to need compiler support.
May 23 2017
I like it, and it seems like it would nicely generalize if there are more cases that we find need this treatment.
May 22 2017
Counterproposal: in -std=*++14 onwards, treat this construct as a user-defined literal, but fall back on the built-in interpretation if name lookup doesn't find an operator""i function. (The two interpretations only conflict if the source code explicitly does something like using namespace std::complex_literals;, which seems like a pretty big clue that the user expected 1.0ito be std::complex<double> rather than _Complex double.)