- User Since
- Jul 12 2012, 2:19 PM (292 w, 6 d)
Tue, Feb 20
Mon, Feb 19
Sun, Feb 18
LGTM, but I'd also like @rjmccall's opinion.
Fri, Feb 16
Thu, Feb 15
Wed, Feb 14
Tue, Feb 13
Please handle this by temporarily updating the This pointer appropriately when evaluating the CXXDefaultInitExpr rather than trying to work around the This value being wrong later (your approach will still go wrong if the This pointer is used in other ways, such as an explicit mention of this or a member function call). You can refer to how we do this in CodeGen: look for CodeGenFunction::CXXDefaultInitExprScope and CodeGenFunction::FieldConstructionScope.
Mon, Feb 12
Sat, Feb 10
Fri, Feb 9
How do you avoid suppressing diagnostics in the declaration of a template that is not a partial specialization? For example:
Thanks! I'd noticed this weirdness but wasn't sure what we could do about it without breaking MS compat. I like this approach a lot.
Thu, Feb 8
Wed, Feb 7
I defined _LIBCPP_BUILDING_LIBRARY instead to match the existing code in libc++abi (stdlib_exception.cpp, stdlib_new_delete.cpp), which uses the _BUILDING_LIBRARY macro as a general way to say "give me all the symbols because I'm building the library". That seemed to match the semantics of this code better than a macro whose purpose is "I am writing user code that is still using this removed symbol".
Tue, Feb 6
Looks fine to me.
Mon, Feb 5
The Itanium C++ ABI specifies a convention of using the source-level syntax in the mangling of vendor extensions. This gives a fairly natural naming convention for such extensions. That would suggest that the identifier to use here is __swiftcall__, not __swift_cc / __Swift. Given that the MS ABI mangles return types even for non-template functions, one consistent place to put this marker would be on the return type. That is, mangle __attribute__((__swiftcall__)) T f(...) as if it were __swiftcall__<T> f(...), and likewise for function pointer types.
Fri, Feb 2
Thu, Feb 1
Wed, Jan 31
Jan 22 2018
Looks good, thanks!
Jan 20 2018
Shouldn't we be changing only the heterogeneous functions here?
Jan 17 2018
Please take a look at http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0588r1.html, which respecifies the rules for lambda capture and its interaction with default arguments, and has been voted into the C++ working paper as a defect report resolution. The approach there is to use a purely syntactic, scope-based mechanism to detect problems such as this. (In dependent contexts, we can track on the DeclRefExpr whether the name is odr-usable, in case we can't tell whether it's odr-used from the template definition alone.)
Why do we disallow this when converting from double-double to other types? IIUC, ISO C's TS 18661 requires a conversion between _Float128 and other types, so if we have a good reason to not support such for PPC64, we should be doing something more subtle than just treating _Float128 as an alias for __float128.
Jan 16 2018
Did you forget to upload the updated patch? This looks unchanged compared to the prior version.
Jan 15 2018
Jan 12 2018
This will still diagnose valid and reasonable programs, such as:
Jan 11 2018
Please also document this trait in docs/LanguageExtensions.rst.
Committed as r322316.
Jan 10 2018
Jan 8 2018
The lexer is doing the right thing; per the C++ lexical rules, the +1 is not part of the token in this case.
Looks fine. Can you also update include/clang/Basic/AttrDocs.td to mention multiversioning in TargetDocs? Then you can add a few words here to say "consult the documentation for the target attribute for more information" or similar :)
Looks fine to me, wait to see if Eli has more comments.
I'd like to see more testing for the template instantiation case. I don't see any test coverage for the "attribute only affects instantiations whose members are trivial-for-calls" part.
Looks good to me with just a few more tweaks (assuming these comments don't uncover any new issues). Thank you!