[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

erichkeane updated this revision to Diff 472268. erichkeane added a comment. fix clang format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136975/new/ https://reviews.llvm.org/D136975 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticSemaKinds.td clang/inclu

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

erichkeane updated this revision to Diff 472093. erichkeane added a comment. ACTUALLY add the libcxx testing, also rebase so hopefully it applies cleanly. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136975/new/ https://reviews.llvm.org/D136975 Files: clang/docs/ReleaseNotes.rst c

[PATCH] D136975: [Concepts] Correctly handle failure when checking concepts recursively

erichkeane updated this revision to Diff 472086. erichkeane added a comment. Add DELETE_ME so that the libcxx tests run. Due to the nature of this patch, it is possible that I will have missed a few cases where we should be 're-starting' the substitution stack checking. CHANGES SINCE LAST ACT

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

erichkeane requested changes to this revision. erichkeane added a comment. This revision now requires changes to proceed. I think the 'this results in a hard error, not failed lookup' is a necessity here based on discussions on the core reflector. Also see: https://reviews.llvm.org/D133052 Re

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

erichkeane added a comment. In D133052#3896663 , @luken-google wrote: > Because of the change to `InsertNode` to not assert on already cached > concepts this patch now works. If I try to catch the recursive expansion on a > higher level the test in `co

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:282 + StringRef FirstComponentName = Path[0].first->getName(); + if (!getSourceManager().isInSystemHeader(Path[0].second) && + (FirstComponentName == "std" || ChuanqiXu wrote: > aa

[PATCH] D136886: [clang] ASTImporter: Fix importing of va_list types and declarations

erichkeane added inline comments. Comment at: clang/lib/Sema/Sema.cpp:460-462 +Scope S(TUScope, Scope::DeclScope, getDiagnostics()); +PushDeclContext(&S, DC); +PushOnScopeChains(ND, &S); mizvekov wrote: > aaron.ballman wrote: > > Is it val

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

erichkeane added inline comments. Comment at: clang/docs/ReleaseNotes.rst:347 Fixes `Issue 57562 `_. +- Clang now diagnoses use of invalid or reserved module names. Both are + diagnosed as an error, but the diagnostic is supp

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

erichkeane added a comment. In D133052#3892650 , @erichkeane wrote: > In D133052#3891768 , @usaxena95 > wrote: > >> My suggestion would be to properly handle cycles in >> `CheckConstraintSatisfaction`. This pro

[PATCH] D133052: [clang] Avoid crash when expanding conversion templates in concepts.

erichkeane added a comment. In D133052#3891768 , @usaxena95 wrote: > My suggestion would be to properly handle cycles in > `CheckConstraintSatisfaction`. This problem goes beyond cycles introduced by > conversion. See #53213

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:257 +else if (PartName.startswith("std") && + (PartName.size() == 3 || isDigit(PartName.drop_front(3)[0]))) + Reason = /*reserved*/ 1; cor3ntin wrote: > erichkeane wr

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:257 +else if (PartName.startswith("std") && + (PartName.size() == 3 || isDigit(PartName.drop_front(3)[0]))) + Reason = /*reserved*/ 1; aaron.ballman wrote: > cor3ntin

[PATCH] D136953: [C++20] Diagnose invalid and reserved module names

erichkeane added a comment. I'll leave it to the modules experts to decide whether they're happy with this, but I had a drive-by. Also, I see none of the tests validate 'import', just 'export'. Based on the standard, BOTH are illegal, right :D Comment at: clang/lib/Sema/Sem

[PATCH] D136942: [C2x] Add test coverage for WG14 N2322

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. I'm OK with this either way, I think you've proved out the important cases. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136942/new/ https://reviews.llvm.org/D136942 ___

[PATCH] D136942: [C2x] Add test coverage for WG14 N2322

erichkeane added a comment. I had two suggestions for more tests, but this is probably covering everything. Comment at: clang/test/C/C2x/n2322.c:44 + ) == 2007); +} + I might suggest: _Static_assert(555 == __\ LI\ NE\ __) Which, is basically exactly what the

[PATCH] D134128: [P0857R0 Part B] Resubmit an implemention for constrained template template parameters

erichkeane added a comment. In D134128#3890447 , @h-vetinari wrote: > Congratulations for landing this! > > How far do you (both) think we're away from completing concepts? Are the > issues (aside from CWG1496 & CWG1734) mentioned on > https://clang.ll

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

erichkeane added a comment. I noticed in a downstream that this changes how we emit structs to IR sometimes (see https://godbolt.org/z/74xeq9rTj for an example, but the 'no-opaque-pointers' isn't necessary to cause this, just anything that causes emission of the struct). Are we OK with that?

[PATCH] D134128: [P0857R0 Part B] Resubmit an implemention for constrained template template parameters

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGae48d1c76a6c: [P0857R0 Part-B] Allows `require' clauses appearing in (authored by lime, committed by erichkeane). Changed

[PATCH] D134128: [P0857R0 Part B] Resubmit an implemention for constrained template template parameters

erichkeane added a comment. In D134128#3888361 , @lime wrote: > `Liming Liu ` > > It is also included in the patch file. Ok, I'll use that. Typically we don't use a separately attached patch-file, we just use the phabricator 'download patch', so I didn

[PATCH] D134128: [P0857R0 Part B] Resubmit an implemention for constrained template template parameters

erichkeane added a comment. In D134128#3887549 , @lime wrote: > Update as suggested. Could you please help me apply this patch? It seems > unsuitable to grant a fresh account the accessibility. > > F25074741: 0001-P0857R0-Part-B-Allows-require-clauses-ap

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

erichkeane added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:11725 +TreeTransform::TransformUnspecifiedValueExpr(UnspecifiedValueExpr *E) { + return E; +} aaron.ballman wrote: > @erichkeane -- is this correct, or does work need to be done to ins

[PATCH] D134128: [P0857R0 Part B] Resubmit an implemention for constrained template template parameters

erichkeane added inline comments. Comment at: clang/docs/ReleaseNotes.rst:554 +- Implemented `P0857R0 `_, + which words for constrained lambdas and constrained template *template-parameter*\s. ---

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

erichkeane added a subscriber: aaron.ballman. erichkeane added a comment. Also, as discussed with @aaron.ballman, 'unspecified' has VERY specific meaning in C/C++, and you don't seem to mean that here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D136737: [Draft] [clang] Add builtin_unspecified_value

erichkeane added a comment. It doesn't really make sense to me why this is an AST node expr-node-type rather than just being a call to a builtin (that'll likely need to take a var as a parameter)? We can make that builtin result in a freeze/poison value if necessary. So something like: `_si

[PATCH] D134128: [P0857R0 Part B] Resubmit an implemention for constrained template template parameters

erichkeane added a comment. ALSO: When you commit this: please try to use a more descriptive commit message, more similar to what I posted in Phab originally. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134128/new/ https://reviews.llvm.org/D1341

[PATCH] D134128: [P0857R0 Part B] Resubmit an implemention for constrained template template parameters

erichkeane accepted this revision. erichkeane added a comment. I'm happy with this once the release-note is clarified and all libcxx pre-commit works with it. Comment at: clang/docs/ReleaseNotes.rst:554 +- Implemented `P0857R0

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added a comment. Still need release notes! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134128/new/ https://reviews.llvm.org/D134128 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D136533: [clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access

erichkeane added a comment. In D136533#3883089 , @mizvekov wrote: > In D136533#3883048 , @erichkeane > wrote: > >> One thing you might try is seeing if this is a libcxx-specific thing >> instead, and try doing a

[PATCH] D136533: [clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access

erichkeane added a comment. In D136533#3882976 , @mizvekov wrote: > I don't have access to a macOS machine, and I don't believe any of the > pre-commit CI machines are running it either. > So I don't have much to go on here. > > How do you think we shall

[PATCH] D133874: [clang] Changes to produce sugared converted template arguments

erichkeane accepted this revision. erichkeane added inline comments. This revision is now accepted and ready to land. Comment at: clang/include/clang/Sema/Sema.h:8285 + TemplateArgument &SugaredConverted, + Templ

[PATCH] D133874: [clang] Changes to produce sugared converted template arguments

erichkeane added a comment. The test changes are a little bizarre, are there any better tests you can write that shows this behavior? Also, at the 'end' of this patch set, we should make sure we have a detailed release note. Comment at: clang/include/clang/Sema/Sema.h:8285 +

[PATCH] D133874: [clang] Changes to produce sugared converted template arguments

erichkeane added a comment. In D133874#3882101 , @mizvekov wrote: > @erichkeane since you reviewed and approved 5 patches that depend on this > one, can you also have a look at this please? > > Thanks! Thanks for the ping, I missed this is in the massiv

[PATCH] D136533: [clang] Fix missing diagnostic of declaration use when accessing TypeDecls through typename access

erichkeane accepted this revision. erichkeane added a comment. I'm happy, thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136533/new/ https://reviews.llvm.org/D136533 ___ cfe-commits mailing list c

[PATCH] D136602: NFC: [clang] Template argument cleanups.

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. LGTM once that one overload is removed (the non const one). I'd rather we justify at that point WHY we need that overload in a separate review. Comment at: clang/in

[PATCH] D136451: GH58368: Correct concept checking in a lambda defined in concept

erichkeane added a comment. Reapplied after revert: https://reviews.llvm.org/rGb876f6e2f28779211a829d7d4e841fe68885ae20 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136451/new/ https://reviews.llvm.org/D136451 ___

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added a comment. In D134128#3879295 , @erichkeane wrote: > I'm reasonably OK with this, I'm disappointed the 'skip for specialization' > is what was required, but I don't think I know of a better way. I'll hold > off approving this until I'

[PATCH] D136602: NFC: [clang] Template argument cleanups.

erichkeane added a comment. I like this cleanup in general, just 2 questions. Comment at: clang/include/clang/AST/TemplateBase.h:592 + llvm::ArrayRef arguments() const { return Arguments; } + llvm::MutableArrayRef arguments() { return Arguments; } Ooh, real

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added a comment. ALSO; this is missing the cxx_status.html and release-notes, so please re-add those! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134128/new/ https://reviews.llvm.org/D134128 __

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added a comment. I'm reasonably OK with this, I'm disappointed the 'skip for specialization' is what was required, but I don't think I know of a better way. I'll hold off approving this until I've confirmed that libcxx tests + libcxx-modules-tests are properly passing as a result of

[PATCH] D134604: [clang] Implement sugared substitution changes to infrastructure

erichkeane accepted this revision. erichkeane added a comment. A couple of nits, otherwise I don't have to see this again. Comment at: clang/include/clang/AST/TemplateName.h:158 + // When true the substitution will be finalized (subst node won't be placed). + bool getFinal()

[PATCH] D136566: [clang] Instantiate concepts with sugared template arguments

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:1079 MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs( - CSE->getNamedConcept(), /*Final=*/false, &TAL, + CSE->getNamedConcept(), /*Final=*/true, &TAL, /*Relative

[PATCH] D136451: GH58368: Correct concept checking in a lambda defined in concept

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb7c922607c5b: GH58368: Correct concept checking in a lambda defined in concept (authored by erichkeane). Herald added a project: clang. Changed prio

[PATCH] D136566: [clang] Instantiate concepts with sugared template arguments

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. This is an improvement, thanks! Comment at: clang/lib/Sema/SemaConcept.cpp:1079 MultiLevelTemplateArgumentList MLTAL = S.getTemplateInstantiationArgs( - CSE->

[PATCH] D136533: Fix missing diagnostic of declaration use when accessing TypeDecls through typename access

erichkeane accepted this revision. erichkeane added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:176 + auto *FoundRD = dyn_cast(TD); + if (DCK != DiagCtorKind::None && LookupRD && FoundRD && + FoundRD->isInjectedClassName() && So I know our cod

[PATCH] D136545: [Clang] use non-template function declaration for constraints partial ordering

erichkeane accepted this revision. erichkeane added inline comments. Comment at: clang/test/CXX/over/over.match/over.match.best/p2.cpp:10 }; - bool k1 = A() < A(); // not ordered by constraints: prefer non-rewritten form - bool k2 = A() < A(); // prefer more-constrained 'op

[PATCH] D136451: GH58368: Correct concept checking in a lambda defined in concept

erichkeane updated this revision to Diff 469696. erichkeane added a comment. Woops, forgot to update my test! Do it so we can get tests to run. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136451/new/ https://reviews.llvm.org/D136451 Files: clang/include/clang/AST/ASTNodeTraverser.

[PATCH] D134453: Disambiguate type names when printing NTTP types

erichkeane added subscribers: mizvekov, erichkeane. erichkeane added a comment. FWIW, i find the GCC diagnostics (and the application of this patch) to be much more clear/easy to read. The pile of `{` and `}` don't really look useful/readable/meaningful to me, and leaves us ambiguous, so I'm in

[PATCH] D136451: GH58368: Correct concept checking in a lambda defined in concept

erichkeane added inline comments. Comment at: clang/include/clang/AST/DeclTemplate.h:3310 +// during constraint checking. +class ConceptSpecializationDecl final +: public Decl, aaron.ballman wrote: > Would it make sense to rename this to `ImplicitConceptSpeci

[PATCH] D136451: GH58368: Correct concept checking in a lambda defined in concept

erichkeane updated this revision to Diff 469693. erichkeane marked 4 inline comments as done. erichkeane added a comment. Changes as Aaron suggested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136451/new/ https://reviews.llvm.org/D136451 Files: clang/include/clang/AST/ASTNodeTrave

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added a comment. So I've been messing around with this a bit, and am somewhat confident that IsAtLeastAsConstrainedAs should just contain: unsigned Depth1 = CalculateTemplateDepthForConstraints(*this, D1); unsigned Depth2 = CalculateTemplateDepthForConstraints(*this, D2); f

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:513-534 + llvm::Optional MLTAL = + SetupConstraintCheckingTemplateArgumentsAndScope( + const_cast(FD), {}, Scope); + Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; if (aut

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:513-534 + llvm::Optional MLTAL = + SetupConstraintCheckingTemplateArgumentsAndScope( + const_cast(FD), {}, Scope); + Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; if (aut

[PATCH] D136451: GH58368: Correct concept checking in a lambda defined in concept

erichkeane created this revision. erichkeane added a reviewer: aaron.ballman. Herald added a subscriber: arphaman. Herald added a project: All. erichkeane requested review of this revision. As that bug reports, the problem here is that the lambda's 'context-decl' was not set to the concept, and th

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:513-534 + llvm::Optional MLTAL = + SetupConstraintCheckingTemplateArgumentsAndScope( + const_cast(FD), {}, Scope); + Qualifiers ThisQuals; CXXRecordDecl *Record = nullptr; if (aut

[PATCH] D136133: [Clang] update cxx_dr_status.html by running make_cxx_dr_status

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. SGTM with teh changes discussed above Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136133/new/ https://reviews.llvm.org/D136133 __

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added a comment. In D134128#3865063 , @lime wrote: >> which isn't clear to me what you mean > > - In the function `Sema::CheckTemplateArgument` at line 5725, `Params` has > been substituted in a way all `TemplateTypeParmDecl`s are instantiated

[PATCH] D135772: Stop evaluating trailing requires clause after overload resolution

This revision was automatically updated to reflect the committed changes. Closed by commit rG4bcb85c638c1: Stop evaluating trailing requires clause after overload resolution (authored by erichkeane). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D136133: [Clang] update cxx_dr_status.html by running make_cxx_dr_status

erichkeane added a comment. Thank you so much for doing this. I'm hopeful our hackery can go away sometime soon. Comment at: clang/test/CXX/drs/dr25xx.cpp:3 -namespace dr2565 { // dr252: 16 +namespace dr2565 { // dr2565: partial template If you could add

[PATCH] D136084: Fix LIT CodeGen/Func-attr.c

erichkeane added inline comments. Comment at: clang/test/CodeGen/func-attr.c:1 -// RUN: %clang -c -ffast-math -emit-llvm -S -o - %s \ -// RUN: | FileCheck %s +// RUN: %clang -c -O2 -target x86_64 -ffast-math\ +// RUN: -emit-llvm -S -o - %s | FileCheck %s zahiraa

[PATCH] D136084: Fix LIT CodeGen/Func-attr.c

erichkeane added inline comments. Comment at: clang/test/CodeGen/func-attr.c:1 -// RUN: %clang -c -ffast-math -emit-llvm -S -o - %s \ -// RUN: | FileCheck %s +// RUN: %clang -c -O2 -target x86_64 -ffast-math\ +// RUN: -emit-llvm -S -o - %s | FileCheck %s It isn'

[PATCH] D135772: Stop evaluating trailing requires clause after overload resolution

erichkeane updated this revision to Diff 468185. erichkeane marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135772/new/ https://reviews.llvm.org/D135772 Files: clang/include/clang/Sema/Sema.h clang/lib/Sema/SemaDeclCXX.cpp clang/lib/Sema/SemaExpr.cpp

[PATCH] D135772: Stop evaluating trailing requires clause after overload resolution

erichkeane marked an inline comment as done. erichkeane added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:6748 + SourceLocation ConstraintFailLoc = NakedFn->getBeginLoc(); + ChuanqiXu wrote: > It looks like `ConstraintFailLoc` is not used? Woops!

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added a comment. In D134128#3860993 , @lime wrote: > I think I located the problem. In the line 4014 of the file > `SemaTemplateInstantiateDecl.cpp`, the requires clause is not instantiated as > the parameters of the template parameter list,

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. Daisy's bug ended up being more complicated than I expected... there is a lot to unpack here. In the meantime, I've captured the bug here: https://github.com/llvm/llvm-project/issues/58368 and will continue looking at it. Repository: rG LLVM Github Monorepo CHA

[PATCH] D135919: [Clang] Set thread_local Itanium ABI guard variables before calling constructors.

erichkeane added a comment. So this logically looks/sounds right to me (and I have no code concerns)... But I'm also notoriously bad at anything threading related. I'd love it if one of the other reviewers could sanity check this for me. Repository: rG LLVM Github Monorepo CHANGES SINCE L

[PATCH] D134928: [Sema] Don't treat a non-null template argument as if it were null.

erichkeane accepted this revision. erichkeane added a comment. I would love it if the diagnostic had a little more info why it isn't, but this is probably good enough for now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134928/new/ https://revie

[PATCH] D135772: Stop evaluating trailing requires clause after overload resolution

erichkeane updated this revision to Diff 467762. erichkeane marked an inline comment as done. erichkeane added a comment. Thanks for the review @shafik CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135772/new/ https://reviews.llvm.org/D135772 Files: clang/include/clang/Sema/Sema.h

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#382 , @erichkeane wrote: > In D126907#3854983 , @cor3ntin > wrote: > >> @erichkeane I was looking at Daisy meta programming shenanigans and found >> this crashes https

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:590 +// FIXME: may be incomplete +static unsigned CalculateTemplateDepthForConstraints(const Expr *Constr) { lime wrote: > erichkeane wrote: > > I'd like some sort of assert in the cas

[PATCH] D135820: [clang-tblgen] renames Diagnostic.Text to Diagnostic.Summary

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. I'm unopposed. I think the justification isn't particularly clear to me and doesn't seem discussed in that RFC that I can tell, but I don't really have a concern here. Give folks a f

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3854983 , @cor3ntin wrote: > @erichkeane I was looking at Daisy meta programming shenanigans and found > this crashes https://cute.godbolt.org/z/6fWoEheKc - seems related to your > work, probably? It used to work i

[PATCH] D134529: [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

erichkeane added a comment. In D134529#3853036 , @ilya-biryukov wrote: > In D134529#3852990 , @erichkeane > wrote: > >> Note that @BertalanD noticed an issue with what I expect to be this patch: >> https://god

[PATCH] D134529: [C++20][Clang] P2468R2 The Equality Operator You Are Looking For

erichkeane added subscribers: BertalanD, erichkeane. erichkeane added a comment. Note that @BertalanD noticed an issue with what I expect to be this patch: https://godbolt.org/z/Wjb9rsEYG Can someone more knowledgable about this paper please make sure it is an intended change? It seems to me

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a subscriber: Uthkarsh. erichkeane added a comment. In D126907#3852975 , @BertalanD wrote: > (I know this is a very contrived example with `void operator!=`, but that is > what CVise spat out, and the actual failures are related to compa

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3852909 , @BertalanD wrote: > I tried out D135772 , and our build got > significantly farther than before! I unfortunately discovered another piece > of code that pre babdef27c503c

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:706 + std::tie(OldConstr, NewConstr) = + AdjustConstraintDepth::UnifyConstraintDepthFromDecl(*this, Old, OldConstr, + New, NewConstr);

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:590 +// FIXME: may be incomplete +static unsigned CalculateTemplateDepthForConstraints(const Expr *Constr) { I'd like some sort of assert in the case where you don't know what to do he

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3849583 , @erichkeane wrote: > In D126907#3846591 , @erichkeane > wrote: > >> In D126907#3844788 , @BertalanD >> wrote: >> >>> Hi

[PATCH] D135772: Stop evaluating trailing requires clause after overload resolution

erichkeane created this revision. erichkeane added a reviewer: clang-language-wg. Herald added a project: All. erichkeane requested review of this revision. Reported as it showed up as a constriants failure after the deferred instantiation patch, we were checking constraints TWICE after overload r

[PATCH] D135472: [ODRHash] Hash attributes on declarations.

erichkeane added a comment. Most of my concerns change based on the feedback others have given, so after those are dealt with, I'll do another depever view. Comment at: clang/include/clang/Basic/DiagnosticASTKinds.td:852 +def err_module_odr_violation_attribute : Error< + "%q0

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3846591 , @erichkeane wrote: > In D126907#3844788 , @BertalanD > wrote: > >> Hi @erichkeane, >> >> This change broke compilation of this program >> (https://godbolt.org/z/

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

erichkeane accepted this revision. erichkeane added a comment. This LGTM, but please give @davrec time (a few days?) to do another review before merging. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131858/new/ https://reviews.llvm.org/D131858 _

[PATCH] D135500: [Clang] reject bit-fields as instruction operands in Microsoft style inline asm blocks.

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. 1 nit, otherwise LGTM. Comment at: clang/lib/Sema/SemaStmtAsm.cpp:937 for (uint64_t I = 0; I < NumOutputs + NumInputs; ++I) { -if (Exprs[I]->getType()->isBitIn

[PATCH] D135287: Disallow dereferencing of void* in C++.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6685e56ceddf: Disallow dereferencing of void* in C++. (authored by erichkeane). Herald added a project: clang. Changed prior to commit: https://re

[PATCH] D135341: [clang] adds `__reference_constructs_from_temporary`

erichkeane added inline comments. Comment at: clang/include/clang/Basic/DiagnosticCommonKinds.td:397 +def err_reserved_identifier_for_future_use : Error< + "%0 is a compiler-reserved identifier for a future feature">; } I don't think we should diagnose this for

[PATCH] D126907: Deferred Concept Instantiation Implementation Take 2

erichkeane added a comment. In D126907#3844788 , @BertalanD wrote: > Hi @erichkeane, > > This change broke compilation of this program > (https://godbolt.org/z/KrWGvcf8h; reduced from > https://github.com/SerenityOS/ladybird): > > template > constex

[PATCH] D135099: [C2x] Implement support for nullptr and nullptr_t

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. Happy with this, thanks! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135099/new/ https://reviews.llvm.org/D135099 ___ cfe-commit

[PATCH] D135238: [clang] adds copy-constructible type-trait builtins

erichkeane added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.def:528 +TYPE_TRAIT_1(__is_nothrow_copy_constructible, IsNothrowCopyConstructible, KEYCXX) +TYPE_TRAIT_1(__is_trivially_copy_constructible, IsTriviallyCopyConstructible, KEYCXX) TYPE_TRAIT_2(__r

[PATCH] D135339: [clang] makes `__is_destructible` KEYCXX instead of KEYMS

erichkeane added a comment. In D135339#3840422 , @cjdb wrote: > In D135339#3840406 , @erichkeane > wrote: > >> In D135339#3840323 , @cjdb wrote: >> >>> In D135339#3839876

[PATCH] D135339: [clang] makes `__is_destructible` KEYCXX instead of KEYMS

erichkeane added a comment. In D135339#3840323 , @cjdb wrote: > In D135339#3839876 , @erichkeane > wrote: > >> This doesn't cause us to lose this in Microsoft C mode, does it? Otherwise, >> LGTM. > > Why is des

[PATCH] D135238: [clang] adds copy-constructible type-trait builtins

erichkeane added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.def:528 +TYPE_TRAIT_1(__is_nothrow_copy_constructible, IsNothrowCopyConstructible, KEYCXX) +TYPE_TRAIT_1(__is_trivially_copy_constructible, IsTriviallyCopyConstructible, KEYCXX) TYPE_TRAIT_2(__r

[PATCH] D134128: Resubmit an implemention for constrained template template parameters [P0857R0 Part B]

erichkeane requested changes to this revision. erichkeane added a comment. This revision now requires changes to proceed. In D134128#3840232 , @lime wrote: > I updated patch to fix the previous problem that failed to pass unit tests. > And, isn't this pa

[PATCH] D135287: Disallow dereferencing of void* in C++.

erichkeane updated this revision to Diff 465725. erichkeane marked 2 inline comments as done. erichkeane added a comment. Fix based on Aaron's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135287/new/ https://reviews.llvm.org/D135287 Files: clang/docs/ReleaseNotes.rst cla

[PATCH] D135287: Disallow dereferencing of void* in C++.

erichkeane marked 4 inline comments as done. erichkeane added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:6939-6942 + // Note: This uses a different diagnostics group than the C diagnostic + // so that projects that have disabled the ab

[PATCH] D135339: [clang] makes `__is_destructible` KEYCXX instead of KEYMS

erichkeane added a comment. This doesn't cause us to lose this in Microsoft C mode, does it? Otherwise, LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135339/new/ https://reviews.llvm.org/D135339

[PATCH] D135338: [clang] adds move-assignable type-trait builtins

erichkeane added a comment. Once again, concerns about triviality. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135338/new/ https://reviews.llvm.org/D135338 ___ cfe-commits mailing list cfe-commits@list

[PATCH] D135240: [clang] adds move-constructible type-trait builtins

erichkeane added a comment. Same comments on triviality here unfortunately. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135240/new/ https://reviews.llvm.org/D135240 ___ cfe-commits mailing list cfe-com

[PATCH] D135239: [clang] adds copy-assignable type-trait builtins

erichkeane added a comment. Same comment as the previous patch on triviality. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135239/new/ https://reviews.llvm.org/D135239 ___ cfe-commits mailing list cfe-c

[PATCH] D135238: [clang] adds copy-constructible type-trait builtins

erichkeane added a comment. Please make sure these are going to be OK with libc++ here. "Triviality" is a bit of a hard nut, the standards have re-defined what they mean quite a few times, so this ends up being pretty worthless if it doesn't match the 'version' of this check that the library i

[PATCH] D135177: [clang] adds `__is_scoped_enum`, `__is_nullptr`, and `__is_referenceable`

erichkeane accepted this revision. erichkeane added a comment. This revision is now accepted and ready to land. LGTM... Please do the re-ordering separately here, unless it was necessary here. Comment at: clang/lib/Parse/ParseExpr.cpp:1071 REVERTIBLE_TYPE_TRAIT(__is_

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