[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

erichkeane updated this revision to Diff 261922. erichkeane marked 2 inline comments as done. erichkeane added a comment. For PPC64, better emulate structs by passing _ExtInt as an array. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79118/new/ https://reviews.llvm.org/D79118 Files:

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

erichkeane updated this revision to Diff 262093. erichkeane added a comment. Extracted out a function for PPC64's coerce to array. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79118/new/ https://reviews.llvm.org/D79118 Files: clang/lib/Basic/Targets/AArch64.h clang/lib/Basic/Targe

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938 +if (EIT->getNumBits() > 128) + return getNaturalAlignIndirect(Ty, /*ByVal=*/true); + rjmccall wrote: > rjmccall wrote: > > e

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

erichkeane updated this revision to Diff 262190. erichkeane added a comment. Revert PPC64 changes back to direct/indirect. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79118/new/ https://reviews.llvm.org/D79118 Files: clang/lib/Basic/Targets/AArch64.h clang/lib/Basic/Targets/AMDGP

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4938 +if (EIT->getNumBits() > 128) + return getNaturalAlignIndirect(Ty, /*ByVal=*/true); + rjmccall wrote: > erichkeane wrote: > >

[PATCH] D79118: Implement _ExtInt ABI for all ABIs in Clang, enable type for ABIs

erichkeane updated this revision to Diff 262274. erichkeane marked 5 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79118/new/ https://reviews.llvm.org/D79118 Files: clang/lib/Basic/Targets/AArch64.h clang/lib/Basic/Targets/AMDGPU.h clang/lib/Basic/Targets/A

[PATCH] D79755: Implement constexpr BinaryOperator for vector types

erichkeane created this revision. erichkeane added reviewers: eli.friedman, RKSimon, aaron.ballman. These operations do member-wise versions of the all of the listed operations. This patch implements all of the binaryoperators for these types. Note that the test is required to use codegen as I co

[PATCH] D77068: [XCore] fix crash on unused inline in EmitTargetMetadata

erichkeane added a comment. It doesn't seem like it would be much work to move the EmitTargetMetadata function into the TargetInfo object (as a virtual function taking a reference to the collection). Essentially, have this take a reference to the collection instead of the 'D' and 'GV'. I say

[PATCH] D79830: Add support of __builtin_expect_with_probability

erichkeane added a comment. Type checking/value checking for this should happen during Sema, not codegen. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2049 +ConstantFP *Confidence_f = dyn_cast(Confidence); +if (!Confidence_f) { + CGM.Error(E->getArg(2)->getLocStart(

[PATCH] D79830: Add support of __builtin_expect_with_probability

erichkeane added a comment. In D79830#2034400 , @davidxl wrote: > Is it possible to overload __builtin_expect(..)? No OP, but... First, Overloading builtins is a bit of a pain. You end up having to do custom type checking. Second, GCC already made the

[PATCH] D79830: Add support of __builtin_expect_with_probability

erichkeane added a comment. In D79830#2034779 , @LukeZhuang wrote: > In D79830#2033367 , @lebedev.ri > wrote: > > > Thanks for working on this. > > Please upload patch with full context. > > > Hi, sorry but I'm no

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane marked 10 inline comments as done. erichkeane added a comment. Patch incoming, I think I got everything. I'll do the error on array of aligned items in a separate patch, hopefully tomorrow AM. Comment at: lib/AST/ASTContext.cpp:2183-2184 + for (const auto *Field :

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane updated this revision to Diff 124490. erichkeane marked an inline comment as done. https://reviews.llvm.org/D39347 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h lib/AST/ASTContext.cpp lib/AST/Type.cpp lib/Sema/SemaExprCXX.cpp test/SemaCXX/type-traits.cpp I

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane added inline comments. Comment at: lib/AST/ASTContext.cpp:2238-2240 + // All other pointers are unique. + if (Ty->isPointerType() || Ty->isMemberPointerType()) +return true; rsmith wrote: > erichkeane wrote: > > rsmith wrote: > > > This is not co

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane updated this revision to Diff 124609. erichkeane added a comment. Add has padding to CXXABI getMemberPointerWidthAndAlign to correct padding behavior here. https://reviews.llvm.org/D39347 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h lib/AST/ASTContext.cpp l

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane added inline comments. Comment at: lib/AST/ASTContext.cpp:2183-2184 + for (const auto *Field : RD->fields()) { +if (!Context.hasUniqueObjectRepresentations(Field->getType())) + return false; +int64_t FieldSizeInBits = rsmith wrote: > eric

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane added inline comments. Comment at: lib/AST/ASTContext.cpp:2183-2184 + for (const auto *Field : RD->fields()) { +if (!Context.hasUniqueObjectRepresentations(Field->getType())) + return false; +int64_t FieldSizeInBits = rsmith wrote: > eric

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane added inline comments. Comment at: lib/AST/ASTContext.cpp:2213 +Field->isBitField() +? Field->getBitWidthValue(Context) +: Context.toBits(Context.getTypeSizeInChars(Field->getType())); erichkeane wrote: > rsmith wrote: >

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane updated this revision to Diff 124679. erichkeane added a comment. thanks for the comments rsmith, think this is all caught up to your comments. Also added the test for member ptr differences. https://reviews.llvm.org/D39347 Files: include/clang/AST/ASTContext.h include/clang/AS

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane added a comment. Did ore looking into the bitfield shennangins. Will update this patch tomorrow. @rsmith if you get a chance, do you perhaps have an opinion on 'bool' fields? It seems to me that a bool could either be considered an 8 bit value, or a 1 bit value with 7 bits of padd

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane added inline comments. Comment at: lib/AST/ASTContext.cpp:2213 +Field->isBitField() +? Field->getBitWidthValue(Context) +: Context.toBits(Context.getTypeSizeInChars(Field->getType())); erichkeane wrote: > rsmith wrote: >

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane updated this revision to Diff 124765. https://reviews.llvm.org/D39347 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h lib/AST/ASTContext.cpp lib/AST/CXXABI.h lib/AST/ItaniumCXXABI.cpp lib/AST/MicrosoftCXXABI.cpp lib/AST/Type.cpp lib/Sema/SemaExprCXX.cpp

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane updated this revision to Diff 124769. erichkeane added a comment. Woops, missed an 'svn add' and lost the member ptr test. Back now! https://reviews.llvm.org/D39347 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h lib/AST/ASTContext.cpp lib/AST/CXXABI.h lib/A

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: lib/AST/ASTContext.cpp:2200 + Layout.getBaseClassOffset(Base->getAsCXXRecordDecl()); + CharUnits BaseSize = Context.getTypeSizeInChars(Base); + if (BaseOffset != CurOffse

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane added inline comments. Comment at: lib/AST/ASTContext.cpp:2200 + Layout.getBaseClassOffset(Base->getAsCXXRecordDecl()); + CharUnits BaseSize = Context.getTypeSizeInChars(Base); + if (BaseOffset != CurOffset) rsmith wrote: > erichkean

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane updated this revision to Diff 124859. erichkeane added a comment. Fixed the 'tail padding' mechanism to work correctly as suggested by @rsmith https://reviews.llvm.org/D39347 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h lib/AST/ASTContext.cpp lib/AST/CXXABI.

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane added inline comments. Comment at: lib/AST/ASTContext.cpp:2200 + Layout.getBaseClassOffset(Base->getAsCXXRecordDecl()); + CharUnits BaseSize = Context.getTypeSizeInChars(Base); + if (BaseOffset != CurOffset) rsmith wrote: > erichkean

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane updated this revision to Diff 124863. erichkeane added a comment. Fix for trailing padding BITS, which obviously won't be merged. https://reviews.llvm.org/D39347 Files: include/clang/AST/ASTContext.h include/clang/AST/Type.h lib/AST/ASTContext.cpp lib/AST/CXXABI.h lib/AST/I

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

erichkeane added a comment. In https://reviews.llvm.org/D39347#940015, @rsmith wrote: > Thanks for your patience, this turned out to be surprisingly complicated. Of course! Thank you as well for your input, I was surprised at how complicated this ended up being as well! https://reviews.llvm

[PATCH] D39347: Fix __has_unique_object_representations based on rsmith's input

This revision was automatically updated to reflect the committed changes. Closed by commit rC319446: Fix __has_unique_object_representations implementation (authored by erichkeane). Repository: rC Clang https://reviews.llvm.org/D39347 Files: include/clang/AST/ASTContext.h include/clang/AS

[PATCH] D40673: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2-26

This revision was automatically updated to reflect the committed changes. Closed by commit rC319703: Add _Float128 as alias to __float128 to enable compilations on Fedora27/glibc2… (authored by erichkeane). Repository: rC Clang https://reviews.llvm.org/D40673 Files: include/clang/Basic/Toke

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

erichkeane created this revision. GCC's attribute 'target', in addition to being an optimization hint, also allows function multiversioning. We currently have the former implemented, this is the latter's implementation. This works by enabling functions with the same name/signature to coexist, so

[PATCH] D38596: Implement attribute target multiversioning

erichkeane abandoned this revision. erichkeane added a comment. Completely superceededby this patch here: https://reviews.llvm.org/D40819 Since that one is a complete redesign/reimplementation, I'd prefer to do it separately. https://reviews.llvm.org/D38596 _

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

erichkeane marked 14 inline comments as done. erichkeane added a comment. Incoming patch! Comment at: include/clang/Basic/Attr.td:1809 bool DuplicateArchitecture = false; + bool operator ==(const ParsedTargetAttr &Other) { +return DuplicateArchitecture == Ot

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

erichkeane updated this revision to Diff 125779. erichkeane marked 3 inline comments as done. erichkeane added a comment. Fix all rsmith's and craig's fixes, AFAIK. https://reviews.llvm.org/D40819 Files: include/clang/AST/Decl.h include/clang/Basic/Attr.td include/clang/Basic/DiagnosticSe

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

This revision was automatically updated to reflect the committed changes. Closed by commit rL320391: For Linux/gnu compatibility, preinclude if the file is available (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D34158?vs=123613&id=126386#toc Repository: rL LLV

[PATCH] D34158: For Linux/gnu compatibility, preinclude if the file is available

This revision was automatically updated to reflect the committed changes. Closed by commit rC320391: For Linux/gnu compatibility, preinclude if the file is available (authored by erichkeane). Changed prior to commit: https://reviews.llvm.org/D34158?vs=123613&id=126387#toc Repository: rC Cla

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

erichkeane added a comment. Herald added a subscriber: mgrang. Ping! Anyone have time to take a look for me? https://reviews.llvm.org/D40819 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

erichkeane marked 17 inline comments as done. erichkeane added a comment. Patch incoming, Thank you very much for the review @aaron.ballman Comment at: include/clang/Basic/DiagnosticSemaKinds.td:9333-9335 + "multiversion function would have identical mangling to a previous "

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

erichkeane updated this revision to Diff 126781. erichkeane marked 2 inline comments as done. erichkeane added a comment. Fixed all of @aaron.ballman comments. https://reviews.llvm.org/D40819 Files: include/clang/AST/Decl.h include/clang/Basic/Attr.td include/clang/Basic/DiagnosticSemaKin

[PATCH] D40819: Implement Attribute Target MultiVersioning (Improved edition!)

erichkeane updated this revision to Diff 126829. erichkeane added a comment. @aaron.ballman reminded me that this should only work on ELF targets, so this patch enforces that constraint. https://reviews.llvm.org/D40819 Files: include/clang/AST/Decl.h include/clang/Basic/Attr.td include/c

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

erichkeane added a comment. just a pair of minor changes I'd like to see, otherwise this LGTM. Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:103-106 +VisitSubstNonTypeTemplateParmPackExpr(SubstNonTypeTemplateParmPackExpr *E) { + Unexpanded.push_back({E, E->getPa

[PATCH] D132829: [clang][Interp] Handle ImplictValueInitExprs

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. I assume we're going to see more patches in the future in this space as you discover examples/tests for this, but in order to unblock other tests, this is good enough for now. Reposi

[PATCH] D132832: [clang][Interp] Handle missing local initializers better

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:245 + +if (!Init) + return false; Would be nice to test this before the work to allocate Offset? Comment at: clang/lib/AST/Interp/Function.h:163 +

[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

erichkeane added a comment. LGTM, questioning whether we want to do some macro introspection in C mode (as the CORRECT way to use these 1 bit-bitfields is as a bool), but at least this gives the person the ability to disable this warning. Comment at: clang/test/Sema/constant-

[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. 2 nits, otherwise LGTM. Comment at: clang/lib/Sema/SemaChecking.cpp:13080 - S.Diag(InitLoc, diag::warn_impcast_bitfield_precision_constant) -<< PrettyValue <<

[PATCH] D132831: [clang][Interp] Handle SubstNonTypeTemplateParmExprs

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:273 +const SubstNonTypeTemplateParmExpr *E) { + return this->visit(E->getReplacement()); +} Is there nothing special that has to happen when these are reference parameter

[PATCH] D132851: Further update -Wbitfield-constant-conversion for 1-bit bitfield

erichkeane accepted this revision. erichkeane added a comment. Still LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132851/new/ https://reviews.llvm.org/D132851 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.o

[PATCH] D132831: [clang][Interp] Handle SubstNonTypeTemplateParmExprs

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:273 +const SubstNonTypeTemplateParmExpr *E) { + return this->visit(E->getReplacement()); +} tbaeder wrote: > erichkeane wrote: > > Is there nothing special that has to hap

[PATCH] D132877: Downgrade the UAX31 diagnostic to a warning which defaults to an error

erichkeane added a comment. In D132877#3756328 , @efriedma wrote: > If we don't specifically call out the deprecation period in the diagnostic, > usage will grow beyond what we expect. (Most people don't read the release > notes; they'll just see it's

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:860 +} else if (const auto *ND = Unexpanded[I].first.get(); + isa(ND)) { + // Function parameter pack or init-capture pack. This pattern with the init + c

[PATCH] D128095: [clang] Improve diagnostics for expansion length mismatch

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Happy enough, LGTM. Comment at: clang/lib/Sema/SemaTemplateVariadic.cpp:860 +} else if (const auto *ND = Unexpanded[I].first.get(); + isa(ND)) { +

[PATCH] D132918: [clang] Fix a crash in constant evaluation

erichkeane added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:4797 } +// Can't access properties of an incomplete type. +if (!RD->hasDefinition()) { It seems to me that we shouldn't GET to this function with an incomplete type. I sus

[PATCH] D132918: [clang] Fix a crash in constant evaluation

erichkeane added a comment. Also, without a reduced example lit-test, we shouldn't be making changes. Use creduce or something, but PLEASE come back with a lit test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132918/new/ https://reviews.llvm.o

[PATCH] D132952: [Sema] disable -Wvla for function array parameters

erichkeane added inline comments. Comment at: clang/include/clang/Sema/Sema.h:1760 +// Special builder emitting no diagnostics +SemaDiagnosticBuilder(Sema &S) : S(S) {} SemaDiagnosticBuilder(Kind K, SourceLocation Loc, unsigned DiagID, I haven't look

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

erichkeane added a comment. I don't mind the idea, but wonder if some level of RFC/project-wide decision should be made here? WDYT @aaron.ballman ? Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCG

[PATCH] D133072: [clang] fix profiling of template arguments of template and declaration kind

erichkeane added a comment. Generally happy here. Two quick suggestions, otherewise LGTM. Comment at: clang/lib/AST/ASTContext.cpp:5119 // Find the insert position again. -DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos); +[[maybe_unused]] a

[PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. Just did a quick scroll through this (as it is quite large!), but the general idea seems like a fine one to me. I AM concerned about how it interacts with the deferred concepts instantiation that I've been working on (https://reviews.llvm.org/D126907), particularly

[PATCH] D133072: [clang] fix profiling of template arguments of template and declaration kind

erichkeane accepted this revision. erichkeane added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:5119 // Find the insert position again. -DependentTemplateSpecializationTypes.FindNodeOrInsertPos(ID, InsertPos); +[[maybe_unused]] auto *Nothing = +

[PATCH] D131858: [clang] Track the templated entity in type substitution.

erichkeane added a comment. In D131858#3763878 , @mizvekov wrote: > In D131858#3763851 , @erichkeane > wrote: > >> Just did a quick scroll through this (as it is quite large!), but the >> general idea seems like

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

erichkeane added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const char *CrashDirectory = CCGenDiagno

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

erichkeane added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const char *CrashDirectory = CCGenDiagno

[PATCH] D133082: [clang] Implement setting crash_diagnostics_dir through env variable

erichkeane accepted this revision. erichkeane added inline comments. Comment at: clang/lib/Driver/Driver.cpp:5425 Arg *A = C.getArgs().getLastArg(options::OPT_fcrash_diagnostics_dir); - if (CCGenDiagnostics && A) { -SmallString<128> CrashDirectory(A->getValue()); + const

[PATCH] D133262: [clang] Represent __make_integer_seq as alias template in the AST

erichkeane added inline comments. Comment at: clang/lib/AST/DeclTemplate.cpp:257 +return true; + case TemplateDecl::BuiltinTemplate: { +const auto *BT = cast(this); This seems strange to me that ONLY this one builtin is what makes a template a 'type ali

[PATCH] D111283: [clang] template / auto deduction deduces common sugar

erichkeane added a comment. Thanks for your patience, this was a sizable review that it took a while to be able to make time for. A handful of non-functional reviews, but the functionality looks fine. Comment at: clang/lib/AST/ASTContext.cpp:12139 +return X; + assert(de

[PATCH] D130308: [clang] extend getCommonSugaredType to merge sugar nodes

erichkeane accepted this revision. erichkeane added a comment. I've got concerns about the interface not being clear on null-ability, and THIS patch seems to move us toward nulls being disallowed, so I'd like that formalized when possible (either via asserts or, better, by switching to referenc

[PATCH] D133341: [C++] [Coroutines] Prefer aligned (de)allocation for coroutines - implement the option2 of P2014R0

erichkeane added a comment. I'm not a great reviewer for coroutines as I don't quite get them, so a vastly better commit/review message would be GREATLY appreciated. Code seems fine, but I'd love for someone better at coroutines to take a look too. Comment at: clang/lib/Code

[PATCH] D133425: Silence -Wctad-maybe-unsupported stemming from system headers

erichkeane accepted this revision. erichkeane added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Sema/SemaInit.cpp:10303 // Warn if CTAD was used on a type that does not have any user-defined - // deduction guides. - if (!HasAnyDedu

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3777088 , @ldionne wrote: > In D126907#3746477 , @Mordante > wrote: > >> Unfortunately there are a lot of different options and combination of >> options to test libc++. >>

[PATCH] D133499: [clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext.

erichkeane added inline comments. Comment at: clang/lib/AST/ASTDumper.cpp:205 + if (const Decl *D = dyn_cast(this)) +D->dump(); +} One thing to note is that the 'else' case here is a little uninformative. See https://clang.llvm.org/doxygen/DeclBase_8cpp_so

[PATCH] D133500: [clang] Correct handling of lambdas in lambda default arguments in dependent contexts.

erichkeane added a comment. Patch seems sound, just a nit that we don't need a bunch of the asserts here, since the 'get' function already asserts it looks. I'd like others to take a look too, but this looks acceptable to me. Comment at: clang/lib/Sema/SemaTemplateInstantiat

[PATCH] D133499: [clang]: Add DeclContext::dumpDecl() in order to conveniently dump an AST from a DeclContext.

erichkeane added inline comments. Comment at: clang/lib/AST/ASTDumper.cpp:205 + if (const Decl *D = dyn_cast(this)) +D->dump(); +} aaron.ballman wrote: > erichkeane wrote: > > One thing to note is that the 'else' case here is a little uninformative. > > Se

[PATCH] D133641: [Clang] [Sema] Ignore invalid multiversion function redeclarations

erichkeane added a comment. makes sense to me, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133641/new/ https://reviews.llvm.org/D133641 ___ cfe-commits mailing list cfe-commits@lists.llvm.org h

[PATCH] D133941: [clang][Interp] Record item types in InterpStack

erichkeane added a comment. Can you clarify what the intent of this patch is? Perhaps I'm just being slow today, but I don't really get the intent here. Comment at: clang/lib/AST/Interp/InterpStack.h:135 + return PT_Ptr; +else if (std::is_same::value || std::is_same:

[PATCH] D133941: [clang][Interp] Record item types in InterpStack

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/InterpStack.h:161 + return PT_Uint64; +assert(false); + } llvm_unreachable? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133941/new/ ht

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/Context.cpp:134 +llvm::Optional Context::sizeofType(QualType T) const { + if (llvm::Optional ArgT = classify(T)) +return primSize(*ArgT); This function seems pretty large for something you sh

[PATCH] D133634: [clang] Allow vector of BitInt

erichkeane added a comment. There was no motivation for disabling this other than not wanting/caring enough to define the ABI and writing sufficient tests. While I don't think there is sufficient testing here around ABI/etc, I have no problem with this. Repository: rG LLVM Github Monorepo

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:289 +QualType ArgType = E->getTypeOfArgument(); +return this->emitConst(E, Ctx.getASTContext().getTypeSize(ArgType)); + } You probably want `getTypeSizeInChars`. `getT

[PATCH] D133934: [clang][Interp] Handle sizeof() expressions

erichkeane requested changes to this revision. erichkeane added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 +return this->emitConst( +E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQua

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: llvm/lib/Support/Unicode.cpp:268 + // hyphen in most terminals. + return Printables.contains(UCS) || UCS == 0x00AD; +} As a nit, this condition might be worth reversing to get the 'fast thing' on the LHS of the sho

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:793 + OutStr.reserve(OutStr.size() + Str.size()); + auto it = reinterpret_cast(Str.data()); + auto end = it + Str.size(); aaron.ballman wrote: > Please fix the clang-tidy and clang-fo

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added a comment. I'm generally OK with this. BUT Aaron/JF/Tom should review it. Comment at: clang/lib/Basic/Diagnostic.cpp:817 +if (isPrintable(*Begin) || isWhitespace(*Begin)) { + OutStr.push_back(*Begin); + ++Begin; OutStream << *Beg

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:837 + llvm::sys::unicode::isFormatting(CodepointValue)) { +OutStr.append(CodepointBegin, CodepointEnd); +continue; What about...? Repository: rG LLVM Github

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:837 + llvm::sys::unicode::isFormatting(CodepointValue)) { +OutStr.append(CodepointBegin, CodepointEnd); +continue; cor3ntin wrote: > cor3ntin wrote: > > erichkea

[PATCH] D108469: Improve handling of static assert messages.

erichkeane added inline comments. Comment at: clang/lib/Basic/Diagnostic.cpp:837 + llvm::sys::unicode::isFormatting(CodepointValue)) { +OutStr.append(CodepointBegin, CodepointEnd); +continue; erichkeane wrote: > cor3ntin wrote: > > cor3nt

[PATCH] D128351: [clang] missing outer template levels when checking template constraints

erichkeane added a comment. So I don't think this is the right fix for this... But I DID check and see that the test case seems to be fixed here: https://reviews.llvm.org/D126907 , which does a lot of rework for this sort of thing. That patch is expected to be complete/ready for submission in t

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 440643. erichkeane added a comment. All tests pass now, I was able to get the template-template checks working correctly, and it passes all the tests I have available. @ChuanqiXu if you could review/run what tests you have, it would be greatly appreciate

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 440712. erichkeane added a comment. Rebased! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907 Files: clang/docs/ReleaseNotes.rst clang/include/clang/AST/Decl.h clang/include/clang/AST/DeclBase.h

[PATCH] D128750: [c++20] Implement P2113R0: Changes to the Partial Ordering of Constrained Functions

erichkeane added a comment. Is there any chance you can validate this against https://reviews.llvm.org/D126907 as well? We touch similar code, and I'm intending to get that committed in the near future. Otherwise, after a quick look, I don't see anything particualrly bad, other than a lack of

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane marked 9 inline comments as done. erichkeane added a comment. In D126907#3617713 , @ChuanqiXu wrote: > In D126907#3615807 , @erichkeane > wrote: > >> All tests pass now, I was able to get the template-

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane updated this revision to Diff 441020. erichkeane marked 2 inline comments as done. erichkeane added a comment. All but the 1 comment from @ChuanqiXu fixed, not sure what to do about the 'info'. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.or

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/include/clang/AST/Decl.h:1944-1948 + /// For non-templates this value will be NULL, unless this non-template + /// function declaration was declared directly inside of a function template, + /// in which case this will have a

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2f207439521d: Deferred Concept Instantiation Implementation (authored by erichkeane). Herald added a project: clang. Repository: rG LLVM Github Mo

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

erichkeane added subscribers: ChuanqiXu, iains. erichkeane added a comment. I don't see anything to be concerned about here, but I'd like a modules person to take a look. @ChuanqiXu any chance you can confirm here? (or @iains ?) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D128921: [Sema] Merge C++20 concept definitions from different modules in same TU

erichkeane added a comment. I don't see anything to be concerned about here, but I'd like a modules person to take a look. @ChuanqiXu any chance you can confirm here? (or @iains ?) Also, would like to see a release note on this too! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3623120 , @JDevlieghere wrote: > This change triggers an assertion when building an LLDB test case: > > UNREACHABLE executed at > /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/clang/lib/Sema/SemaTem

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Herald added a reviewer: ributzka. I tried the ninja stage2-check-all on my local build with this patch applied and don't reproduce the issue. Is there any way someone could get me the preprocessed source that this crashed on off the servers or something? @JDevlieg

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. @shafik was able to help me figure out how to repro, and I think I've about fixed this, so I'll try another commit soon. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126907/new/ https://reviews.llvm.org/D126907 _

[PATCH] D128119: [clang] enforce instantiation of constexpr template functions

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:4859 << Function; + } else if (!Recursive) { +Function->setInstantiationIsPending(true); This condition seems a little aggressive, and will end

[PATCH] D128059: [Clang] Add a warning on invalid UTF-8 in comments.

erichkeane added inline comments. Comment at: clang/docs/ReleaseNotes.rst:270 This fixes `Issue 55962 `_. +- Added ``-Winvalid-utf8`` which diagnose invalid UTF-8 code unit sequences in + comments. Reposi

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