[PATCH] D57645: [C++2a] Fix PR40576: Turn destroying delete off prior to C++2a. Add -fdestroying-delete

EricWF created this revision. EricWF added reviewers: ckennelly, rsmith. Herald added a project: clang. Defining new names in namespace `std` is technically a non-conforming extensions. This makes it difficult for libc++ to backport C++20 destroying delete. This patch should make it easier for l

[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type

EricWF added a comment. @arphaman Do you mind if I commit my own version of this? Repository: rL LLVM https://reviews.llvm.org/D37104 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commi

[PATCH] D37104: [libc++] PR34298: Change std::function constructor and move assignment operator SFINAE checks to allow std::function with an incomplete return type

EricWF commandeered this revision. EricWF edited reviewers, added: arphaman; removed: EricWF. EricWF added a comment. I committed a different version of this fix in r312890. Commandeering and closing this review. @arphaman Thanks for the initial patch. Repository: rL LLVM https://reviews.llv

[PATCH] D37133: [libc++] Handle object files named *.obj in merge_archives.py

EricWF added a comment. In https://reviews.llvm.org/D37133#860549, @compnerd wrote: > I think we should avoid this logic entirely and use CMake to do this. It's not that easy to do in CMake, and a CMake solution would be limited to only merging in-tree builds together. This script allows mergin

[PATCH] D37133: [libc++] Handle object files named *.obj in merge_archives.py

EricWF added inline comments. Comment at: utils/merge_archives.py:121 files = glob.glob(os.path.join(temp_directory_root, '*.o')) if not files: How about simply using `*.o*`? https://reviews.llvm.org/D37133 ___

[PATCH] D37134: [libc++] Rerun ranlib manually after merging the static libraries

EricWF requested changes to this revision. EricWF added a comment. Ironically I think parts of this change should be moved into `merge_archives.py`. > the ar used in merge_archives.py might not be the right one for the target OK, then we should modify `merge_archives.py` to support optionally s

[PATCH] D36720: [libc++] Prevent stale site configuration headers

EricWF accepted this revision. EricWF added inline comments. This revision is now accepted and ready to land. Comment at: CMakeLists.txt:633 + if (EXISTS "${site_config_path}") +file(REMOVE "${site_config_path}") + endif() Maybe print a warning or a message

[PATCH] D37134: [libc++] Use CMAKE_AR for merging static libraries

EricWF accepted this revision. EricWF added a comment. LGTM, minus inline comments. Comment at: utils/merge_archives.py:97 parser.add_argument( +'-a', '--ar', dest='ar_exe', required=False, +help='The ar executable to use, finds \'ar\' in the path if not gi

[PATCH] D37677: [libc++] implement future synchronization using atomic_flag

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. I agree with the general consensus that we should only make this change if it's significantly faster, and only after we have a test that demonstrates this. Unfortunately I don't reca

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

EricWF added inline comments. Comment at: include/__split_buffer:279 while (__begin_ != __new_begin) -__alloc_traits::destroy(__alloc(), __to_raw_pointer(__begin_++)); +__alloc_traits::destroy(__alloc(), _VSTD::__to_raw_pointer(__begin_++)); } -

[PATCH] D36719: [libc++] Add site config option for ABI macros

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D36719#841499, @smeenai wrote: > There should probably be some documentation for this, but I couldn't think of > the right place; the Using libc++ documentation onl

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

EricWF added inline comments. Comment at: include/memory:1349 is_same< -decltype(__has_construct_test(declval<_Alloc>(), - declval<_Pointer>(), - declval<_Args>()...)), +

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF updated this revision to Diff 115161. EricWF marked an inline comment as done. EricWF added a comment. - Reimplement without rewriting the AST and instead during the substitution during constant expression evaluation and code gen. I still haven't implemented Richards suggestion to reduce

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF added inline comments. Comment at: test/SemaCXX/loc2.cpp:1 +// RUN: %clang_cc1 -std=c++1z -fcxx-exceptions -fexceptions -verify %s +// expected-no-diagnostics Didn't mean to include this file. Comment at: test/SemaCXX/test.cpp:1 +struct

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF added inline comments. Comment at: lib/AST/Expr.cpp:1940 false, false), -InitExprs(C, initExprs.size()), -LBraceLoc(lbraceloc), RBraceLoc(rbraceloc), AltForm(nullptr, true) -{ + InitExprs(C, initExprs.size()), LBraceLoc(lbraceloc), + RBraceLoc

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF updated this revision to Diff 115162. EricWF added a comment. - Remove accidentally committed test files. - Attempt to remove incidental whitespace changes. https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/Decl.h include/clang/AST/Expr.h includ

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF added inline comments. Comment at: docs/LanguageExtensions.rst:2118 +point is the location of the caller. When the builtins appear as part of a +default member initializer, the invocation point is the location of the +constructor or aggregate initialization used to create

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. - List Item In https://reviews.llvm.org/D28212#871034, @smeenai wrote: > @compnerd, @EricWF and I discussed this a bit on IRC yesterday. > > In this particular case, however, I don

[PATCH] D28212: typeinfo: provide a partial implementation for Win32

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. Woops. didn't mean to reject. https://reviews.llvm.org/D28212 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-b

[PATCH] D37871: [ASAN] Add macro denoting availability of new `__asan_handle_no_return()` function.

EricWF created this revision. Libc++abi attempts to use the newly added `__asan_handle_no_return()` when built under ASAN. Unfortunately older versions of compiler-rt do not provide this symbol, and so libc++abi needs a way to detect if `asan_interface.h` actually provides the function. This p

[PATCH] D37872: [libc++abi] Fix ASAN build with older compiler-rt versions.

EricWF created this revision. Herald added a subscriber: dberris. compiler-rt recently added the `__asan_handle_no_return()` function that libc++abi needs to use, however older versions of compiler-rt don't provide this interface and that breaks the libc++abi build. This patch attempts to fix t

[PATCH] D37871: [ASAN] Add macro denoting availability of new `__asan_handle_no_return()` function.

EricWF added a comment. In https://reviews.llvm.org/D37871#871325, @eugenis wrote: > This function was added to the header recently, but it has been provided by > ASan runtime library since the beginning. Why not simply declare it in > libc++abi? I didn't know that. I'll come up with a separa

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

EricWF added inline comments. Comment at: include/deque:1167-1168 allocator_type& __a = __alloc(); -for (iterator __i = begin(), __e = end(); __i != __e; ++__i) -__alloc_traits::destroy(__a, _VSTD::addressof(*__i)); +for (iterator __i = begin(), __e = end();

[PATCH] D37538: [libc++] Remove problematic ADL in container implementations.

EricWF added a comment. @dlj I went ahead and committed the fixes to `std::allocator_traits` in r313324, because I think we agree those are bugs, and I didn't want this discussion to hold up that fix. I hope you don't mind. https://reviews.llvm.org/D37538 ___

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF updated this revision to Diff 115342. EricWF marked 8 inline comments as done. EricWF added a comment. - Cleanup and address the inline comments I added earlier. https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h include/clang/AST/ExprCXX.h

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF updated this revision to Diff 115349. EricWF added a comment. - Improve test. https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/Expr.h include/clang/AST/ExprCXX.h include/clang/AST/RecursiveASTVisitor.h include/clang/AST/Stmt.h include/clang

[PATCH] D35235: [libc++] Replace __sync_* functions with __libcpp_atomic_* functions

EricWF accepted this revision. EricWF added inline comments. Comment at: src/include/atomic_support.h:153 +_ValueType old; +old = *__target; +*__target = __value; Initialize `old` on the same line it's declared on. https://reviews.llvm.org/D35235

[PATCH] D44865: [libc++] Implement P0608R3 - A sane variant converting constructor

EricWF added inline comments. Comment at: include/variant:1128 +template +struct __overload +: __overload_bool<__overload<_Types...>, bool const volatile> {}; Do we even support volatile types in variant? Repository: rCXX libc++ CHANGES SINCE LAST ACTIO

[PATCH] D51262: Implement P0553 and P0556

EricWF added a comment. Herald added a subscriber: dexonsmith. Please use Clang format on these changes. Otherwise the implementation looks great, I've left some nits. Comment at: include/bit:190 +template +struct __bitop_unsigned_integer +: public integral_constant && +

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

EricWF closed this revision. EricWF added a comment. Committed in r367263. Thanks for the change, sorry about the delay. I'll go watch the bots now. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62977/new/ https://reviews.llvm.org/D62977 _

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

EricWF added a comment. Generally this LGTM. I'll take another pass after the comments are addressed. Comment at: clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp:19 + +static const llvm::StringRef CheckMessage = +"Googletest APIs named with 'case' are de

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

EricWF added inline comments. Comment at: clang-tools-extra/clang-tidy/google/GoogleTidyModule.cpp:30 #include "UnnamedNamespaceInHeaderCheck.h" #include "UsingNamespaceDirectiveCheck.h" Just tried building this. I think you're missing an include for `Upgrad

[PATCH] D48680: Add missing visibility annotation for __base

EricWF added a comment. You have an ODR violation. You can't compile one TU with `-fno-rtti -fno-exceptions` and another without. You give the definitions of `__base` different vtables. Repository: rCXX libc++ CHANGES SINCE LAST ACTION https://reviews.llvm.org/D48680/new/ https://reviews

[PATCH] D62977: [clang-tidy]: Google: new check 'google-upgrade-googletest-case'

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. I don't have any other comments. LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62977/new/ https://reviews.llvm.org/D62977 ___ cfe

[PATCH] D51262: Implement P0553 and P0556

EricWF added a comment. I would like to see a version of this after the inline comments are addressed. Comment at: libcxx/include/bit:193 + is_unsigned_v<_Tp> && +!is_same_v, bool> && +!is_same_v, char> && `_IsNotSame` is faster and bett

[PATCH] D51262: Implement P0553 and P0556

EricWF accepted this revision. EricWF added inline comments. This revision is now accepted and ready to land. Comment at: libcxx/include/bit:240 +else if constexpr (sizeof(_Tp) <= sizeof(unsigned long)) +return __clz(static_cast(__t)) + - (numeric_limits:

[PATCH] D61365: [libcxx] [test] Suppress float->int narrowing warning in vector range-construction test.

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. This LGTM after correctly wrapping it as Casey mentionse. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61365/new/ https://reviews.llvm.org/D61365 _

[PATCH] D61366: [libcxx] [test] Don't assert that moved-from containers with non-POCMA allocators are empty.

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. I'm not sure I agree with your design decision, but this patch LGTM. Are you not allowed to move the containers elements in this case? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

EricWF added a comment. From Billy and my last discussion, I think we came to the agreement that it's not clear exactly what the "standard behavior" is. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61364/new/ https://reviews.llvm.org/D61364 _

[PATCH] D61364: [libcxx] [test] Fix incorrect allocator::construct assertions in associative container tests

EricWF added inline comments. Comment at: test/std/containers/map_allocator_requirement_test_templates.h:236 const ValueTp v(42, 1); -cc->expect(); +cc->expect(); It ret = c.insert(c.end(), std::move(v)); I really think the current behavior libc

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF marked 14 inline comments as done. EricWF added a comment. Address review comments. Comment at: lib/AST/ExprConstant.cpp:3370-3371 + + assert((!Base || !isa(Base)) && + "Base should have already been transformed into a StringLiteral"); + rsmith

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF updated this revision to Diff 199893. EricWF marked an inline comment as done. EricWF added a comment. Merge with upstream. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/ASTContex

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

This revision was automatically updated to reflect the committed changes. Closed by commit rC360937: Implement __builtin_LINE() et. al. to support source location capture. (authored by EricWF, committed by ). Herald added a subscriber: kristina. Herald added a project: clang. Repository: rC Cla

[PATCH] D62429: Fix linkage of _ZTS strings for types involving incomplete classes to match the Itanium ABI rules.

EricWF added a comment. I think this is llvm.org/PR37398 I tried fixing this a while back in r332028 but the fix got reverted for causing llvm.org/PR37545. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D62429/new/ https://reviews.llvm.org/D62429 _

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

EricWF marked an inline comment as done. EricWF added inline comments. Comment at: docs/LibASTMatchersReference.html:2579-2581 +y(x); Matches +NS::y(x); Doesn't match +y(42); Doesn't match. aaron.ballman wrote: > EricWF wrote: > > aaron.ballman wrote:

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

EricWF updated this revision to Diff 177907. EricWF marked an inline comment as done. EricWF added a comment. Address final review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55534/new/ https://reviews.llvm.org/D55534 Files: docs/LibASTMatchersReference.html include/cla

[PATCH] D55534: [AST] Store "UsesADL" information in CallExpr.

This revision was automatically updated to reflect the committed changes. Closed by commit rC348977: [AST] Store "UsesADL" information in CallExpr. (authored by EricWF, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55534/new/ https://reviews.llvm.o

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

EricWF added inline comments. Comment at: clang-tidy/abseil/MakeUniqueCheck.cpp:28 + recordType(hasDeclaration(classTemplateSpecializationDecl( + hasName("::std::unique_ptr"), templateArgumentCountIs(2), + hasTemplateArgument( Does this cat

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

EricWF marked an inline comment as done. EricWF added a comment. I'm not sure how you're building this, but it doesn't link for me. We need to add a dependency on `clangTidyModernizeModule` since we're deriving from its `MakeSmartPtrCheck`. Comment at: clang-tidy/abseil/MakeU

[PATCH] D40218: [Clang] Add __builtin_launder

EricWF updated this revision to Diff 178267. EricWF added a comment. Merging with upstream. Preparing to commit. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40218/new/ https://reviews.llvm.org/D40218 Files: include/clang/Basic/Builtins.def include/clang/Basic/DiagnosticSemaKinds.

[PATCH] D40218: [Clang] Add __builtin_launder

This revision was automatically updated to reflect the committed changes. Closed by commit rC349195: [Clang] Add __builtin_launder (authored by EricWF, committed by ). Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D40218/new/ https://reviews.llvm.org/D40218 Files:

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF updated this revision to Diff 179211. EricWF added a comment. Herald added a reviewer: shafik. Merge with upstream. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/ASTContext.h in

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF added a comment. Ping! I would like to land this before the next release. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D55500: [Builtins] Implement __builtin_is_constant_evaluated for use in C++2a

EricWF marked an inline comment as done. EricWF added a comment. Ping! I would like to land this before the next release. @rsmith, what sorts of additional tests are needed? Comment at: include/clang/Basic/Builtins.def:759 +// Random C++ builtins. +LANGBUILTIN(__builtin_is_con

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF marked 13 inline comments as done. EricWF added a comment. Address review comments. Updated patch coming shortly. Comment at: include/clang/AST/Expr.h:4147 + SourceLocation getBeginLoc() const LLVM_READONLY { return BuiltinLoc; } + SourceLocation getEndLoc() const LLVM

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF updated this revision to Diff 180597. EricWF marked 4 inline comments as done. EricWF added a comment. Address review comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37035/new/ https://reviews.llvm.org/D37035 Files: docs/LanguageExtensions.rst include/clang/AST/ASTCo

[PATCH] D37035: Implement __builtin_LINE() et. al. to support source location capture.

EricWF marked 18 inline comments as done. EricWF added a comment. Mark more review comments as done. Comment at: lib/AST/Expr.cpp:2026-2027 + // human readable name. + if (IdentifierInfo *II = FD->getDeclName().getAsIdentifierInfo()) +return MkStr(II->getNameS

[PATCH] D50106: [libc++] Fix tuple assignment from type derived from a tuple-like

EricWF requested changes to this revision. EricWF added a comment. This revision now requires changes to proceed. Our constructors still have the same bug,. Are you planning on fixing those as well? Doing so will require a metric butt-tonne of overloads. If you're not planning on fixing the const

[PATCH] D67052: Add reference type transformation builtins

EricWF added inline comments. Comment at: clang/include/clang/AST/Type.h:4353 +RemoveReferenceType, +AddRValueType, +AddLValueType This should say ref in the name. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1094 +return DeclSp

[PATCH] D67052: Add reference type transformation builtins

EricWF added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1099 + +void Parser::ParseAddReferenceTypeSpecifier(DeclSpec &DS) { + DeclSpec::TST ReferenceTransformTST = ReferenceTransformTokToDeclSpec(); I think this should be generalized and merge

[PATCH] D45015: [Preprocessor] Allow libc++ to detect when aligned allocation is unavailable.

EricWF added a comment. In https://reviews.llvm.org/D45015#1064922, @vsapsai wrote: > Another approach is `__has_feature` but I don't think it is applicable in > this case. > > Is there a way right now to detect that aligned allocation is supported by > clang, regardless of link time? Asking to

[PATCH] D45529: [CMake] Set the default ABI version for Fuchsia in CMake as well

EricWF added a comment. OK, so we need to bump the SO version -- However I don't want to generate a __site_config header because of that if the __config header is sufficient. I would like to see a solution that takes that into account. Repository: rCXX libc++ https://reviews.llvm.org/D45529

[PATCH] D45596: [libcxx] [test] Avoid unary_function.

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. I think this would have been a good candidate for post-commit review. https://reviews.llvm.org/D45596 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D45595: [libcxx] [test] Fix nodiscard warnings.

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. There's a macro for this in test_macros.h to make it easier to search for. please use that https://reviews.llvm.org/D45595 ___ cfe-commits maili

[PATCH] D45594: [libcxx] [test] Silence MSVC warning C4146.

EricWF accepted this revision. EricWF added a comment. This revision is now accepted and ready to land. This would have been fine for post-commit review. https://reviews.llvm.org/D45594 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 4 inline comments as done. EricWF added inline comments. Comment at: include/clang/AST/ComparisonCategories.h:68 + /// \brief A map containing the comparison category values built from the + /// standard library. The key is a value of ComparisonCategoryValue. + l

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 4 inline comments as done. EricWF added inline comments. Comment at: include/clang/AST/Expr.h:3097-3106 + bool isRelationalOp() const { +return isRelationalOp(getOpcode()) || + (getOpcode() == BO_Cmp && IsCmpOrdered); + } static bool isEqualityO

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 5 inline comments as done. EricWF added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9377-9378 +def err_implied_comparison_category_type_not_found : Error< + "%0 type was not found; include before defining " + "or using 'operator<=>'">;

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 7 inline comments as done. EricWF added inline comments. Comment at: lib/AST/ExprConstant.cpp:6238-6263 +bool VisitBinCmp(const BinaryOperator *E) { + using CCR = ComparisonCategoryResult; + const ComparisonCategoryInfo &CmpInfo = + Info.Ctx.C

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9384-9385 +def err_spaceship_comparison_of_void_ptr : Error< + "three-way comparison with void pointer %select{operand type|operand types}0 " + "(%1 and %2)">; +def err_spaceship_comparison_of

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp: +bool Sema::BuildComparisonCategoryData(SourceLocation Loc) { + using CCKT = ComparisonCategoryKind; rsmith wrote: > EricWF wrote: > > rsmith wrote: > > > If you put this on Sema, you'll n

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: include/clang/AST/ComparisonCategories.h:77-78 + /// comparison category. For example 'std::strong_equality::equal' + const DeclRefExpr *getResultValue(ComparisonCategoryResult ValueKind) const { +const DeclRefExpr *DR = getResult

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 30 inline comments as done. EricWF added inline comments. Comment at: include/clang/AST/ComparisonCategories.h:194 + + const ASTContext &Ctx; + mutable llvm::DenseMap Data; Storing the `ASTContext` in `ComparisonCategories` and `ComparisonCategory

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 24 inline comments as done. EricWF added inline comments. Comment at: lib/AST/ExprConstant.cpp:3784 +static bool EvaluateVarDecl(EvalInfo &Info, const VarDecl *VD, +APValue *Dest = nullptr) { // We don't need to evaluate the initialize

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 4 inline comments as done. EricWF added inline comments. Comment at: lib/Sema/SemaExpr.cpp:9928 +// result is of type std::strong_equality +if (CompositeTy->isFunctionPointerType() || +CompositeTy->isMemberPointerType() || CompositeTy->isNullPtrType(

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: lib/Sema/SemaExpr.cpp:9928 +// result is of type std::strong_equality +if (CompositeTy->isFunctionPointerType() || +CompositeTy->isMemberPointerType() || CompositeTy->isNullPtrType()) EricWF wrote: > rsmit

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 3 inline comments as done. EricWF added inline comments. Comment at: lib/Sema/SemaExpr.cpp:9816 +RHS = S.ImpCastExprToType(RHS.get(), Type, CK_BitCast); + } else { +// C++2a [expr.spaceship]p4 rsmith wrote: > We still need to apply the usua

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF created this revision. EricWF added a reviewer: rsmith. Herald added a subscriber: mgrang. This is a work-in-progress attempt to add `operator<=>` rewriting. It's nowhere close to complete, but I would like some initial feedback on the direction. As currently implemented, rewritten and sy

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF added a comment. I'm guessing this implementation is super non-conforming, since expressions are evaluated during overload resolution. My plan is to keep the general shape of this patch intact, but fix the eager building of expressions with something less wasteful. Repository: rC Cla

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF planned changes to this revision. EricWF added a comment. I know how to do better. Comment at: include/clang/Sema/Overload.h:938 +/// exited. +struct RewrittenCandidateContextGuard { + RewrittenCandidateContextGuard(OverloadCandidateSet &CS)

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF marked 6 inline comments as done. EricWF added inline comments. Comment at: lib/Sema/SemaOverload.cpp: + bool PerformADL) { + assert(getLangOpts().CPlusPlus2a); + auto Opc = BinaryOperator::getOverloadedOpcode(Op); ---

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF marked 2 inline comments as done. EricWF added inline comments. Comment at: lib/Sema/SemaOverload.cpp:12537-12565 +/// Rewritten candidates have been added but not checked for validity. They +/// could still be non-viable if: +/// (A) The rewritten call (x <=> y) is a bui

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF marked an inline comment as done. EricWF added inline comments. Comment at: lib/Sema/SemaOverload.cpp:9218-9219 +// --- F2 is a rewritten candidate ([over.match.oper]) and F1 is not. +if (Cand2.getRewrittenKind() && !Cand1.getRewrittenKind()) + return true; +

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF added inline comments. Comment at: lib/Sema/SemaOverload.cpp:9218-9219 +// --- F2 is a rewritten candidate ([over.match.oper]) and F1 is not. +if (Cand2.getRewrittenKind() && !Cand1.getRewrittenKind()) + return true; +if (Cand1.getRewrittenKind() && Cand2.

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF added inline comments. Comment at: lib/Sema/SemaOverload.cpp:9218-9219 +// --- F2 is a rewritten candidate ([over.match.oper]) and F1 is not. +if (Cand2.getRewrittenKind() && !Cand1.getRewrittenKind()) + return true; +if (Cand1.getRewrittenKind() && Cand2.

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF added inline comments. Comment at: lib/Sema/SemaOverload.cpp:9218-9219 +// --- F2 is a rewritten candidate ([over.match.oper]) and F1 is not. +if (Cand2.getRewrittenKind() && !Cand1.getRewrittenKind()) + return true; +if (Cand1.getRewrittenKind() && Cand2.

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: lib/CodeGen/CGExprAgg.cpp:1002 + return EmitFinalDestCopy( + E->getType(), CGF.MakeNaturalAlignAddrLValue(Select, E->getType())); +} rjmccall wrote: > Is there something in Sema actually validating that the comparis

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

EricWF marked 12 inline comments as done. EricWF added a comment. Ping. https://reviews.llvm.org/D45131 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: lib/CodeGen/CGExprAgg.cpp:1002 + return EmitFinalDestCopy( + E->getType(), CGF.MakeNaturalAlignAddrLValue(Select, E->getType())); +} EricWF wrote: > rjmccall wrote: > > Is there something in Sema actually validating

[PATCH] D45131: [AST] Refactor UnaryTransformType into TransformTraitType supporting non-unary transforms

EricWF planned changes to this revision. EricWF marked 7 inline comments as done. EricWF added inline comments. Comment at: include/clang/ASTMatchers/ASTMatchers.h:5262-5266 +AST_MATCHER(Type, unaryTransformType) { + if (const auto *T = Node.getAs()) +return T->getNumArgs()

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: lib/Sema/SemaDeclCXX.cpp:8931 + /*ConstArg*/ true, false, false, false, false); + auto *CopyCtor = cast_or_null(SMI.getMethod()); + rjmccall wrote: > Sorry, I didn't realize you'd go off and writ

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added a comment. In https://reviews.llvm.org/D45476#1087446, @cfe-commits wrote: > I think you and Richard agreed that you weren’t going to synthesize a whole > expression tree at every use of the operator, and I agree with that > decision. That’s very different from what I’m asking you

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added a comment. OK, As I see it, we have two choices: (1) Stash the expressions in Sema and import them like In https://reviews.llvm.org/D45476#1088047, @rjmccall wrote: > > Ah. If you want to be able to find this thing without a Sema around in order > to > handle deserialized ex

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked an inline comment as done. EricWF added inline comments. Comment at: lib/CodeGen/CGExprAgg.cpp:971 + auto EmitCmpRes = [&](const VarDecl *VD) { +return CGF.CGM.GetAddrOfGlobalVar(VD); + }; rsmith wrote: > Perhaps directly emit the constant val

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 18 inline comments as done. EricWF added inline comments. Comment at: include/clang/AST/ASTContext.h:1986 + /// This object needs to be initialized by Sema the first time it checks + /// a three-way comparison. + ComparisonCategories CompCategories; -

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 12 inline comments as done. EricWF added inline comments. Comment at: lib/Sema/SemaExpr.cpp:9825 +LHS = S.ImpCastExprToType(LHS.get(), IntType, CK_IntegralCast); +RHS = S.ImpCastExprToType(RHS.get(), IntType, CK_IntegralCast); +LHSType = RHSType = IntTyp

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF marked 8 inline comments as done. EricWF added inline comments. Comment at: include/clang/AST/ComparisonCategories.h:71 + /// standard library. The key is a value of ComparisonCategoryResult. + mutable llvm::DenseMap Objects; + rjmccall wrote: > EricWF w

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: lib/CodeGen/CGExprAgg.cpp:964 +RHS = CGF.EmitAnyExpr(E->getRHS()).getAggregatePointer(); +break; + case TEK_Complex: EricWF wrote: > rjmccall wrote: > > It looks like we don't actually support any aggregate types

[PATCH] D45476: [C++2a] Implement operator<=> CodeGen and ExprConstant

EricWF added inline comments. Comment at: lib/AST/ExprConstant.cpp:8829 + return EvaluateComparisonBinaryOperator(Info, E, OnSuccess, [&]() { +return ExprEvaluatorBaseTy::VisitBinaryOperator(E); + }); rsmith wrote: > It'd be clearer to call `VisitBinCmp` ra

[PATCH] D45680: [C++2a] Add operator<=> Rewriting - Early Attempt

EricWF updated this revision to Diff 145390. EricWF marked 7 inline comments as done. EricWF added a comment. - Remove the `= default` changes as requested. - Attempt to work around ambiguity in overload resolution caused by differing lvalue or qualification conversions when ranking synthesized

<    1   2   3   4   5   6   7   8   9   10   >