[PATCH] D138802: [clang][Interp] Implement DecompositionDecls

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:291 /// Returns whether we should create a global variable for the /// given VarDecl. + bool isGlobalDecl(const ValueDecl *VD) const { Comment at:

[PATCH] D137487: [clang][Interp] Start implementing builtin functions

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.h:1315 + if (InterpretBuiltin(S, PC, Func->getBuiltinID())) { +NewFrame.release(); +return true; We don't have to update `S.Current`? Comment at:

[PATCH] D139173: [clang] Add test for CWG600

shafik added inline comments. Comment at: clang/test/CXX/drs/dr6xx.cpp:18 + sp->f(2); + sp->f(2.2); // expected-error {{is a private member}} +} Maybe add a comment above this saying something like: ``` // access control is applied after overload resolution

[PATCH] D139173: [clang] Add test for CWG600

shafik added inline comments. Comment at: clang/test/CXX/drs/dr6xx.cpp:18 + sp->f(2); + sp->f(2.2); // expected-error {{is a private member}} +} Endill wrote: > aaron.ballman wrote: > > Endill wrote: > > > Endill wrote: > > > > shafik wrote: > > > > > Maybe

[PATCH] D139173: [clang] Add test for CWG600

shafik added a comment. Thank you for updating the DR statuses! This is much appreciated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139173/new/ https://reviews.llvm.org/D139173 ___ cfe-commits

[PATCH] D139172: [clang] Mark CWG554 as N/A

shafik added a comment. @aaron.ballman do you think it is worth it to provide a link to `p1787` as well? I know you can just goto the issue and see that but it feels helpful. I actually missed this at first b/c I usually goto end of the issue to look for the resolution and was confused.

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added a comment. It looks like we have an existing message for the regular function case e.g.: void g() { int x; void f(int y = x); auto lambda1 = [] {}; } see godbolt: https://godbolt.org/z/1vneM5b35 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D139429: [clang] Add test for CWG418

shafik added a comment. I think the relevant change from p1787 is to dcl.fct.default p4 , specifically: > ... Declarations that inhabit different scopes have completely distinct

[PATCH] D139429: [clang] Add test for CWG418

shafik added a comment. In D139429#3974491 , @cor3ntin wrote: > I'm not sure how I feel about this. > Clang does not, implement correctly that paragraph at all, so I think the > best course here is to create an issue on the llvm github and mark the dr

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added a comment. It looks like the existing diagnostic is issued by `CheckDefaultArgumentVisitor`. I think you can reuse it here but not 100% on that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139400/new/

[PATCH] D139541: Fix parameter name in Sema::addInitCapture to ByRef.

shafik added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:892-894 + LSI->addCapture(Var, /*isBlock*/ false, ByRef, /*isNested*/ false, Var->getLocation(), SourceLocation(), Var->getType(), /*Invalid*/ false);

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:11 +#include // massberg #include "TreeTransform.h" Please remove. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139400/new/

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/test/SemaTemplate/default-template-arguments.cpp:9 + + auto lambda1 = [] {}; // expected-error {{default argument references local variable x of enclosing function}} + auto lambda2 = [] {}; To clarify my

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:1594 + if (VarDecl *VD = dyn_cast(DRE->getDecl())) { +if (VD->isLocalVarDecl()) { + Diag(DRE->getLocation(), So if we look at

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

shafik added a comment. If I am reading the code correctly it looks like if the call to `(*I)->isValueDependent()` is true then the temporary will not be created and therefore we should not be attempting to access the slot. If this is the case then maybe the checking in

[PATCH] D139485: [LLVM] Remove redundant .c_str() and .get() calls where they are not needed.

shafik added inline comments. Comment at: lldb/source/API/SBPlatform.cpp:14 #include "lldb/API/SBLaunchInfo.h" -#include "lldb/API/SBPlatform.h" #include "lldb/API/SBUnixSignals.h" These redundant header removals looks unrelated and should be done as a

[PATCH] D139705: [clang] fix zero-initialization fix-it for variable template

shafik added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:3870-3872 +} +if (const ASTTemplateArgumentListInfo *Info = VTSD->getTemplateArgsInfo()) + EndLoc = Info->getRAngleLoc(); If I understand correctly we either have a

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

shafik added a comment. In D139713#3989709 , @Pierre-vh wrote: > In D139713#3989071 , @shafik wrote: > >> If I am reading the code correctly it looks like if the call to >> `(*I)->isValueDependent()` is true

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:1594 + if (VarDecl *VD = dyn_cast(DRE->getDecl())) { +if (VD->isLocalVarDecl()) { + Diag(DRE->getLocation(), massberg wrote: > shafik wrote: > > So if we look at

[PATCH] D139429: [clang] Add test for CWG418

shafik added a comment. In D139429#3974745 , @Endill wrote: >> I think the relevant change from p1787 is to dcl.fct.default p4, >> specifically: >> >>> ... Declarations that inhabit different scopes have completely distinct >>> sets of default

[PATCH] D139429: [clang] Add test for CWG418

shafik added a subscriber: aaron.ballman. shafik added a comment. In D139429#3975028 , @Endill wrote: > I opened #59363 on bug > tracker. > What should I do about this patch then? I believe

[PATCH] D139429: [clang] Add test for CWG418

shafik added a comment. In D139429#3975223 , @Endill wrote: > @shafik does this imply that example from [over.match.best]/4 should be > included in this patch? Yes, I believe we should. We are only conforming if we get all of the cases correct.

[PATCH] D139172: [clang] Mark CWG554 as N/A

shafik accepted this revision. shafik added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139172/new/ https://reviews.llvm.org/D139172 ___ cfe-commits mailing list

[PATCH] D129531: [clang][C++20] P0960R3 and P1975R0: Allow initializing aggregates from a parenthesized list of values

shafik added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:4760 + : Expr(CXXParenListInitExprClass, T, + T->isLValueReferenceType() ? VK_LValue + : T->isRValueReferenceType() ? VK_XValue It looks like we use this

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/test/SemaTemplate/default-template-arguments.cpp:12 + auto lambda1 = [] {}; // expected-error {{default argument references local variable x_constexpr of enclosing function}} + auto lambda2 = [] {}; // expected-error {{default

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

shafik added a comment. This looks good to me but I see at least one concern that Aaron had that he did not get back on, so I will wait for him to approve. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:835 IsTemporary = true; Ty = E->getType(); }

[PATCH] D136815: [clang][Interp] Unify visiting variable declarations

shafik accepted this revision. shafik added a comment. LGTM Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1218 +auto GlobalIndex = P.getGlobal(VD); +assert(GlobalIndex); // visitVarDecl() didn't return false. +if (!this->emitGetPtrGlobal(*GlobalIndex, VD))

[PATCH] D137070: [clang][Interp] Support destructors

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:30 /// Scope used to handle temporaries in toplevel variable declarations. -template class DeclScope final : public LocalScope { +template class DeclScope final : public VariableScope {

[PATCH] D137070: [clang][Interp] Support destructors

shafik added inline comments. Comment at: clang/test/AST/Interp/cxx20.cpp:279 + static_assert(testInc2() == 1, ""); +}; CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137070/new/ https://reviews.llvm.org/D137070

[PATCH] D137235: [clang][Interp] Fix ImplicitValueInitExprs for pointer types

shafik accepted this revision. shafik added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137235/new/ https://reviews.llvm.org/D137235 ___ cfe-commits mailing list

[PATCH] D137232: [clang][Interp] Support inc/dec operators on pointers

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

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.h:918 return false; + if (!Ptr.isRoot()) +Ptr.initialize(); tbaeder wrote: > shafik wrote: > > Can you explain what is going on here? Why do we need to initialize if it > > is not

[PATCH] D137070: [clang][Interp] Support destructors

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137070/new/ https://reviews.llvm.org/D137070 ___ cfe-commits mailing list

[PATCH] D136694: [clang][Interp] Check that constructor calls initialize all record fields

shafik accepted this revision. shafik added a comment. LGTM Comment at: clang/lib/AST/Interp/Interp.cpp:1 //===--- Interpcpp - Interpreter for the constexpr VM --*- C++ -*-===// // One more time :-) CHANGES SINCE LAST ACTION

[PATCH] D137415: [clang][Interp] Implement switch statements

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:405 + const Expr *Cond = S->getCond(); + PrimType CondT = this->classifyPrim(Cond->getType()); + The condition could be a class type w/ a conversion operator, this does not

[PATCH] D140455: [Clang] Diagnose undefined behavior in a constant expression while evaluating a compound assignment with remainder as operand

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane, tahonermann. Herald added a project: All. shafik requested review of this revision. Currently we don't diagnose overflow in a constant expression for the case of compound assignment with remainder as a operand. In

[PATCH] D137071: [clang][Interp] Implement missing compound assign operators

shafik accepted this revision. shafik added a comment. LGTM Comment at: clang/test/AST/Interp/literals.cpp:553 + static_assert(IntRem(2, 1) == 0, ""); + static_assert(IntRem(9, 7) == 2, ""); + aaron.ballman wrote: > tbaeder wrote: > > aaron.ballman wrote: >

[PATCH] D139261: [Clang] Modify sanity check assert in AggExprEmitter::VisitInitListExpr to cover anonymous struct in a union GNU extension

shafik updated this revision to Diff 484613. shafik added a comment. - Add Release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139261/new/ https://reviews.llvm.org/D139261 Files: clang/docs/ReleaseNotes.rst clang/lib/CodeGen/CGExprAgg.cpp

[PATCH] D139261: [Clang] Modify sanity check assert in AggExprEmitter::VisitInitListExpr to cover anonymous struct in a union GNU extension

shafik added inline comments. Comment at: clang/lib/CodeGen/CGExprAgg.cpp:1688 for (const auto *Field : record->fields()) -assert(Field->isUnnamedBitfield() && "Only unnamed bitfields allowed"); +assert((Field->isUnnamedBitfield() ||

[PATCH] D139261: [Clang] Modify sanity check assert in AggExprEmitter::VisitInitListExpr to cover anonymous struct in a union GNU extension

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG475cc44a2cba: [Clang] Modify sanity check assert in AggExprEmitter::VisitInitListExpr to… (authored by shafik). Herald added a project: clang.

[PATCH] D138914: Make evaluation of nested requirement consistent with requires expr.

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:2339 +Req->getConstraintExpr()->getSourceRange(), Satisfaction)) + TransConstraint = Result[0]; +assert(!Trap.hasErrorOccurred() && "Substitution failures must be handled "

[PATCH] D139261: [Clang] Modify sanity check assert in AggExprEmitter::VisitInitListExpr to cover anonymous struct in a union GNU extension

shafik added a comment. I realize I am modifying codegen but the test is in sema, I am happy to move the test to somewhere more appropriate if someone has a good suggestion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139261/new/ https://reviews.llvm.org/D139261

[PATCH] D139261: [Clang] Modify sanity check assert in AggExprEmitter::VisitInitListExpr to cover anonymous struct in a union GNU extension

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane. Herald added a project: All. shafik requested review of this revision. `AggExprEmitter::VisitInitListExpr` sanity checks that an empty union is really empty and not a semantic analysis failure. The assert is missing

[PATCH] D139429: [clang] Add test for CWG418

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM, thank you for all your efforts. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139429/new/ https://reviews.llvm.org/D139429 ___

[PATCH] D138091: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as NonType when they are brought into scope via using enum

shafik added a comment. Adding some more reviewers to get more eyes who may be familiar with this corner for the code. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138091/new/ https://reviews.llvm.org/D138091 ___ cfe-commits mailing list

[PATCH] D138194: [clang][Parser][NFC] Simplify ParseParenExprOrCondition

shafik added inline comments. Comment at: clang/lib/Parse/ParseStmt.cpp:1286 if (getLangOpts().CPlusPlus) { -Cond = ParseCXXCondition(InitStmt, Loc, CK, MissingOK); +Cond = ParseCXXCondition(InitStmt, Loc, CK, false); } else { Nitpick edit for

[PATCH] D138822: [clang] Add test for CWG36

shafik added inline comments. Comment at: clang/test/CXX/drs/dr0xx.cpp:454 + +namespace dr36 { // dr36: yes +namespace example1 { aaron.ballman wrote: > It took me a while to convince myself, but yes, I agree that Clang seems to > implement the DR. I had to go

[PATCH] D138603: [Clang] Implement LWG3823 for __is_aggregate

shafik added inline comments. Comment at: clang/test/SemaCXX/type-traits.cpp:556 static_assert(__is_aggregate(EmptyArMB), ""); static_assert(!__is_aggregate(void), ""); static_assert(!__is_aggregate(const volatile void), ""); Should this be `true` now

[PATCH] D138270: [clang][Sema] Skip checking int expressions for overflow in constexpr functions

shafik added a comment. We may not have good code coverage, what about a case like this: constexpr void f() { int arr[10]{}; arr[1024*1024*1024*1204]; } do you still obtain: error: constexpr function never produces a constant expression [-Winvalid-constexpr] constexpr

[PATCH] D138091: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as NonType when they are brought into scope via using enum

shafik marked an inline comment as done. shafik added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:1250-1251 bool ADL = UseArgumentDependentLookup(SS, Result, NextToken.is(tok::l_paren)); - if (Result.isSingleResult() && !ADL && !FirstDecl->isCXXClassMember()) +

[PATCH] D138091: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as NonType when they are brought into scope via using enum

shafik updated this revision to Diff 478630. shafik marked an inline comment as done. shafik added a comment. Add release note. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138091/new/ https://reviews.llvm.org/D138091 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaDecl.cpp

[PATCH] D138091: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as NonType when they are brought into scope via using enum

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG54be300f7e0b: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as… (authored by shafik). Herald added a project: clang.

[PATCH] D138947: [Clang] Remove invalid assert from Sema::BuildCXXTypeConstructExpr

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane, davide, rsmith. Herald added a project: All. shafik requested review of this revision. Currently `Sema::BuildCXXTypeConstructExpr` asserts that list initialization must mean we have an `InitListExpr` as well. We

[PATCH] D138947: [Clang] Remove invalid assert from Sema::BuildCXXTypeConstructExpr

shafik added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:1464 - (Exprs.size() == 1 && isa(Exprs[0]))) && - "List initialization must have initializer list as expression."); SourceRange FullRange = SourceRange(TyBeginLoc, RParenOrBraceLoc);

[PATCH] D138947: [Clang] Remove invalid assert from Sema::BuildCXXTypeConstructExpr

shafik added a comment. The windows failure happened on the build before as well so not related to this change: https://buildkite.com/llvm-project/premerge-checks/builds/123890 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138947/new/ https://reviews.llvm.org/D138947

[PATCH] D138091: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as NonType when they are brought into scope via using enum

shafik added a comment. In D138091#3930356 , @urnathan wrote: > Thanks, I didn;'t know about ClassifyName, and obviously never hit a need to > adjust it. Thanks for fixing. > > The comment above says we don't resolve member-access non-overload sets in

[PATCH] D137897: Extend the number of case Sema::CheckForIntOverflow covers

shafik updated this revision to Diff 475528. shafik added a comment. Add Release note. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137897/new/ https://reviews.llvm.org/D137897 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaChecking.cpp

[PATCH] D137897: Extend the number of case Sema::CheckForIntOverflow covers

This revision was automatically updated to reflect the committed changes. Closed by commit rG9332ddfba69c: [Clang] Extend the number of case Sema::CheckForIntOverflow covers (authored by shafik). Herald added a project: clang. Changed prior to commit:

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

shafik added inline comments. Comment at: clang/lib/AST/Interp/Context.cpp:128 InterpState State(Parent, *P, Stk, *this); - State.Current = new InterpFrame(State, Func, nullptr, {}, {}); + State.Current = new InterpFrame(State, Func, nullptr, {}); if (Interpret(State,

[PATCH] D138947: [Clang] Remove invalid assert from Sema::BuildCXXTypeConstructExpr

shafik updated this revision to Diff 479161. shafik added a comment. Bring back assert but w/ the check for `InitListExpr` removed. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138947/new/ https://reviews.llvm.org/D138947 Files: clang/lib/Sema/SemaExprCXX.cpp

[PATCH] D135750: [clang][Interp] Track initialization state of local variables

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:835 IsTemporary = true; Ty = E->getType(); } Do we really want to the type of the expression? If we have a `ValueDecl` don't we want that type? I think they should

[PATCH] D137706: [clang][Interp] Implement IntegralToPointer casts

shafik added a comment. I agree with @aaron.ballman I expect some more tests for this. Comment at: clang/lib/AST/Interp/Pointer.h:70 Pointer(Block *B, unsigned BaseAndOffset); + Pointer(unsigned Offset); Pointer(const Pointer ); Is the only cast we have

[PATCH] D139148: Fix nullptr dereference found by Coverity static analysis tool

shafik added a comment. This looks like the correct thing to do but I see further down there are also unconditional uses of `Initializer` that should also be guarded with a check. It would be good to fix those as well since we are here. I am guessing the answer is no since this was found using

[PATCH] D138091: [Clang] Fix Sema::ClassifyName so that it classifies EnumConstantDecl as NonType when they are brought into scope via using enum

shafik created this revision. shafik added reviewers: erichkeane, aaron.ballman, urnathan. Herald added a project: All. shafik requested review of this revision. Currently `Sema::ClassifyName(...)` in some cases when an `enumerator` is brought into scope via `using enum` during lookup it can end

[PATCH] D138947: [Clang] Adjust assert from Sema::BuildCXXTypeConstructExpr

shafik updated this revision to Diff 479332. shafik added a comment. - Update assert wording - Add release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138947/new/ https://reviews.llvm.org/D138947 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaExprCXX.cpp

[PATCH] D138947: [Clang] Adjust assert from Sema::BuildCXXTypeConstructExpr

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGef10f81985f6: [Clang] Adjust assert from Sema::BuildCXXTypeConstructExpr (authored by shafik). Herald added a project: clang. Repository: rG LLVM

[PATCH] D140838: [clang] fix crash on generic lambda with lambda in decltype

shafik added a comment. Makes sense to me as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140838/new/ https://reviews.llvm.org/D140838 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D141954: Forbid implicit conversion of constraint expression to bool

shafik added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:379 + S.Context, SubstitutedExpression.get()->getType(), + CK_LValueToRValue, SubstitutedExpression.get(), nullptr, + VK_PRValue, FPOptionsOverride());

[PATCH] D140598: [Clang] Add sanity check in Sema::getDestructorName to prevent nullptr dereference

shafik updated this revision to Diff 491554. shafik marked 2 inline comments as done. shafik added a comment. - Switched to isValid() over isSet() - Added release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140598/new/ https://reviews.llvm.org/D140598 Files:

[PATCH] D138275: [clang][Interp] Avoid leaking init maps of local primitive arrays

shafik added a comment. This looks good to me but I am wondering how "interpretation is interrupted" can happen. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138275/new/ https://reviews.llvm.org/D138275 ___ cfe-commits mailing list

[PATCH] D140845: [clang][Interp] Fix left-/right-shifting more than sizeof(unsigned)

shafik added inline comments. Comment at: clang/test/AST/Interp/shifts.cpp:149 + constexpr signed int R = 62; + constexpr decltype(L) M = L << R; }; Can we add a test for left shift as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D142437: [clang] Add the check of membership in decltype for the issue #58674

shafik added a comment. FYI, I was looking at similar issues over the weekend and put up this PR: https://reviews.llvm.org/D142490 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142437/new/ https://reviews.llvm.org/D142437 ___ cfe-commits

[PATCH] D142490: [Clang] Fix ClassifyImplicitMemberAccess to handle cases where the access in an unevaluated context is not within a CXXRecordDecl or CXXMethodDecl

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane. Herald added a project: All. shafik requested review of this revision. `ClassifyImplicitMemberAccess` assumes that if we are not in a static context then the `DeclContext` must be a `CXXRecordDecl` or a

[PATCH] D142328: [clang][Interp] Fix compound assign operator types

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142328/new/ https://reviews.llvm.org/D142328 ___ cfe-commits mailing list

[PATCH] D142534: Fix emission of consteval constructor of derived type

shafik added a comment. Makes sense to me but I am not familiar with this area so I will let someone else who has more knowledge approve. Comment at: clang/test/CodeGenCXX/cxx20-consteval-crash.cpp:95 +namespace Issue60166 { + nitpick: I have been using

[PATCH] D142534: Fix emission of consteval constructor of derived type

shafik added a comment. I think having a link to the github issue in the summary allows for the issue be closed automatically when you commit. Is this correct @aaron.ballman I have been doing this for a while and they get closed when I commit but maybe there is another mechanism involved.

[PATCH] D140598: [Clang] Add sanity check in Sema::getDestructorName to prevent nullptr dereference

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6ec446ddcee3: [Clang] Add sanity check in Sema::getDestructorName to prevent nullptr… (authored by shafik). Herald added a project: clang. Changed

[PATCH] D129951: adds `__disable_adl` attribute

shafik added inline comments. Comment at: clang/include/clang/Basic/AttrDocs.td:542 If a statement is marked ``nomerge`` and contains call expressions, those call -expressions inside the statement will not be merged during optimization. This +expressions inside the statement

[PATCH] D142604: [Clang] Fix __VA_OPT__ implementation so that it treats the concatenation of a non-placemaker token and placemaker token as a non-placemaker token

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane, hubert.reinterpretcast. Herald added a project: All. shafik requested review of this revision. Currently the implementation of `__VA_OPT__` will treat the concatenation of a non-placemaker token and placemaker token

[PATCH] D142604: [Clang] Fix __VA_OPT__ implementation so that it treats the concatenation of a non-placemaker token and placemaker token as a non-placemaker token

shafik added inline comments. Comment at: clang/test/Preprocessor/macro_vaopt_p1042r1.cpp:1 - RUN: %clang_cc1 -E %s -pedantic -std=c++2a | FileCheck -strict-whitespace %s - I removed Windows lines ending from this test, I don't think they matter but if someone

[PATCH] D142604: [Clang] Fix __VA_OPT__ implementation so that it treats the concatenation of a non-placemaker token and placemaker token as a non-placemaker token

shafik added a comment. It looks like the modules test failures are unrelated and are failing all the recent modules builds. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142604/new/ https://reviews.llvm.org/D142604 ___ cfe-commits mailing

[PATCH] D142867: [Clang] Add machinery to catch overflow in unary minus outside of a constant expression context

shafik created this revision. shafik added reviewers: erichkeane, aaron.ballman. Herald added a project: All. shafik requested review of this revision. We provide several diagnostics for various undefined behaviors due to signed integer overflow outside of a constant expression context. We were

[PATCH] D142316: [clang] Add test for CWG2396

shafik added inline comments. Comment at: clang/test/CXX/drs/dr23xx.cpp:202 + // void g2(A a) { a.operator B decltype(B())::*(); } + // void h(A a) { a.operator identity::type B::*(); } + // void h2(A a) { a.operator B identity::type::*(); } Endill wrote:

[PATCH] D142692: [clang] Store the template param list of an explicit variable template specialization

shafik added subscribers: clang-language-wg, shafik. shafik added a comment. Adding `clang-language-wg` for more visibility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142692/new/ https://reviews.llvm.org/D142692

[PATCH] D142799: [Clang] Fix unconditional access to Attr pointer when checking if _Nullable is applicable to a type

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG2bd8aeea7e7d: [Clang] Fix unconditional access to Attr pointer when checking if _Nullable is… (authored by shafik). Herald added a project: clang.

[PATCH] D142799: [Clang] Fix unconditional access to Attr pointer when checking if _Nullable is applicable to a type

shafik updated this revision to Diff 493351. shafik added a comment. - Add release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142799/new/ https://reviews.llvm.org/D142799 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/TreeTransform.h clang/test/SemaCXX/nullability.cpp

[PATCH] D142799: [Clang] Fix unconditional access to Attr pointer when checking if _Nullable is applicable to a type

shafik updated this revision to Diff 493389. shafik marked an inline comment as done. shafik added a comment. - Fix up release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142799/new/ https://reviews.llvm.org/D142799 Files: clang/docs/ReleaseNotes.rst

[PATCH] D142799: [Clang] Fix unconditional access to Attr pointer when checking if _Nullable is applicable to a type

shafik added a comment. The build failures look unrelated to this change. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142799/new/ https://reviews.llvm.org/D142799 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D142316: [clang] Add test for CWG2396

shafik added inline comments. Comment at: clang/test/CXX/drs/dr23xx.cpp:202 + // void g2(A a) { a.operator B decltype(B())::*(); } + // void h(A a) { a.operator identity::type B::*(); } + // void h2(A a) { a.operator B identity::type::*(); } While gcc

[PATCH] D142604: [Clang] Fix __VA_OPT__ implementation so that it treats the concatenation of a non-placemaker token and placemaker token as a non-placemaker token

shafik updated this revision to Diff 492560. shafik added a comment. - Update line ending in test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142604/new/ https://reviews.llvm.org/D142604 Files: clang/lib/Lex/TokenLexer.cpp clang/test/Preprocessor/macro_vaopt_p1042r1.cpp Index:

[PATCH] D142604: [Clang] Fix __VA_OPT__ implementation so that it treats the concatenation of a non-placemaker token and placemaker token as a non-placemaker token

shafik updated this revision to Diff 492562. shafik added a comment. - I think I correctly updated line endings this time CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142604/new/ https://reviews.llvm.org/D142604 Files: clang/lib/Lex/TokenLexer.cpp

[PATCH] D142639: [clang] Warn by default that implicit capture of 'this' is deprecated in C++20 and later.

shafik accepted this revision. shafik added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142639/new/ https://reviews.llvm.org/D142639 ___ cfe-commits mailing list

[PATCH] D142799: [Clang] Fix unconditional access to Attr pointer when checking if _Nullable is applicable to a type

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane. Herald added a project: All. shafik requested review of this revision. In `TransformAttributedType(...)` when checking if `_Nullable` can be applied to a type it dereferences `TL.getAttr()` unconditionally which we

[PATCH] D142604: [Clang] Fix __VA_OPT__ implementation so that it treats the concatenation of a non-placemaker token and placemaker token as a non-placemaker token

shafik updated this revision to Diff 492589. shafik added a comment. - Apply clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142604/new/ https://reviews.llvm.org/D142604 Files: clang/lib/Lex/TokenLexer.cpp clang/test/Preprocessor/macro_vaopt_p1042r1.cpp Index:

[PATCH] D141775: [Clang] Export CanPassInRegisters as a type trait

shafik added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:5387 + return RD->canPassInRegisters(); +return true; } erichkeane wrote: > Is there good reason to return true for all non-record types? Should we > instead be limiting the

[PATCH] D141784: [clang][Interp] Fix binary comma operators

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141784/new/ https://reviews.llvm.org/D141784 ___ cfe-commits mailing list

[PATCH] D141803: [Clang] Reject in-class defaulting of previously declared comparison operators

shafik added a comment. Interesting it looks like neither gcc nor MSVC diagnose this either but it looks correct to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141803/new/ https://reviews.llvm.org/D141803

[PATCH] D142316: [clang] Add test for CWG2396

shafik added a comment. You mention CWG2385 as na but you don't explain how it was resolved, was it superceded by p1787 . Also nitpick you mentioned `P1787` in the description but it links to a

[PATCH] D142315: [clang] Add test for CWG1111

shafik added inline comments. Comment at: clang/www/cxx_dr_status.html:6476 Remove dual-scope lookup of member template names -Unknown +Clang 6 aaron.ballman wrote: > erichkeane wrote: > > Were you able to track down which patch fixed this in

[PATCH] D142381: [clang] Add test for CWG1960

shafik accepted this revision. shafik added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142381/new/ https://reviews.llvm.org/D142381 ___ cfe-commits mailing list

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