[PATCH] D134859: [clang][Interp] Implement basic support for floating point values

shafik added inline comments. Comment at: clang/lib/AST/Interp/Floating.h:80 + bool isMinusOne() const { return V == -1; } + + ComparisonCategoryResult compare(const Floating &RHS) const { `bool isNaN() const { return V != V; }` CHANGES SINCE LAST ACTION ht

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

shafik added inline comments. Comment at: clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1-11.cpp:31 IP<&tl> ip7; // expected-error{{non-type template argument of type 'int *' is not a constant expression}} +IP<(int*)1> ip8; // expected-error {{non-type template argument does n

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

shafik added inline comments. Comment at: clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1-11.cpp:31 IP<&tl> ip7; // expected-error{{non-type template argument of type 'int *' is not a constant expression}} +IP<(int*)1> ip8; // expected-error {{non-type template argument does n

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

shafik added a comment. It looks like in `Sema::CheckTemplateArgument` in C++17 mode we end up calling `CheckConvertedConstantExpression` which sees a `IntegralToPointer` cast which is basically an reintepret_cast and rejects it as ill-formed. In the C++11 case we end up in `CheckTemplateArgume

[PATCH] D134885: [Clang] Fix variant crashes from GH58028, GH57370

shafik added a comment. In D134885#3826335 , @royjacobson wrote: > Apparently some of the workers crashed with the test - > https://lab.llvm.org/buildbot/#/builders/216/builds/10556, but I couldn't > reproduce this locally. @shafik any idea why the dia

[PATCH] D134334: [Clang] Fix crash in isCXXDeclarationSpecifier when attempting to annotate template name

shafik added a comment. In D134334#3805590 , @erichkeane wrote: > I have no real idea what is going on here, the parser isn't an area where I > spend much time. Can you ELI5? I am going to try but perhaps fail to explain this in more detail and more

[PATCH] D134744: [clang][Interp] Implement rem opcode

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

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

shafik added inline comments. Comment at: clang/test/CXX/temp/temp.arg/temp.arg.nontype/p1-11.cpp:31 IP<&tl> ip7; // expected-error{{non-type template argument of type 'int *' is not a constant expression}} +IP<(int*)1> ip8; // expected-error {{non-type template argument does n

[PATCH] D135012: [clang][Interp] Implement bitwise and operations

shafik added inline comments. Comment at: clang/lib/AST/Interp/Integral.h:215 + static bool band(Integral A, Integral B, unsigned OpBits, Integral *R) { +*R = Integral(A.V & B.V); Maybe `bitAnd`? Comment at: clang/test/AST/Interp/literal

[PATCH] D135025: [clang][Interp] Support base class constructors

shafik added a comment. I realize it is not directly related but I don't see it in the test suite (maybe I missed it) but also delegating constructors, default member initializaers Vs mem-init list (mem-init list has priority) and mem-init list items out of order of initialization. =

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaCast.cpp:2999 + Self.Diag(SrcExpr.get()->getExprLoc(), diag::err_nullptr_cast) + << 0 /*nullptr to type*/ << DestType; + SrcExpr = ExprError(); Curious why put the comment after? When

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:8730 + // result also has that type. + if (LHSTy->isNullPtrType() && Context.hasSameType(LHSTy, RHSTy)) +return ResTy; erichkeane wrote: > what does this do with the GNU ternary-thing?

[PATCH] D135012: [clang][Interp] Implement bitwise and operations

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/AST/Interp/literals.cpp:289 + static_assert((1337 & -1) == 1337, ""); + static_assert((0 & gimme(12)) == 0, ""); +}; Why `gimme

[PATCH] D135025: [clang][Interp] Support base class constructors

shafik added a comment. This looks good to me but I would like one other set of eyes on it CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135025/new/ https://reviews.llvm.org/D135025 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D135169: [LLDB] Fix printing a static bool struct member when using "image lookup -t"

shafik added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:733 + static CXXBoolLiteralExpr *Create(const ASTContext &C, bool Val, QualType Ty, +SourceLocation Loc) { I think this makes sense but if we are go

[PATCH] D135175: [clang] adds `__is_bounded_array` and `__is_unbounded_array` as builtins

shafik added a comment. Looks good but let's have Aaron check it out. Comment at: clang/lib/Sema/SemaExprCXX.cpp:24 #include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/Type.h" #include "clang/AST/TypeLoc.h" Why also `Type.h`? Co

[PATCH] D134749: [clang][Interp] Implement Div opcode

shafik added inline comments. Comment at: clang/test/AST/Interp/literals.cpp:171 + +namespace div { + constexpr int zero() { return 0; } Can we add a test case for `INT_MIN / -1` same for `%` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D135256: [clang] Add Create method for CXXBoolLiteralExpr

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135256/new/ https://reviews.llvm.org/D135256 ___

[PATCH] D135170: [LLDB] Fix crash when printing a struct with a static signed char member

shafik accepted this revision. shafik added a comment. LGTM Comment at: lldb/test/API/lang/cpp/const_static_integral_member/main.cpp:39 const static auto char_min = std::numeric_limits::min(); const static auto uchar_min = std::numeric_limits::min(); We

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

shafik accepted this revision as: shafik. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/docs/ReleaseNotes.rst:92 + error so constraint checking and SFINAE checking can be compatible with other + compilers. It is likely that th

[PATCH] D135361: [clang][Interp] Implement bitwise Or operations

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135361/new/ https://reviews.llvm.org/D135361 ___

[PATCH] D135370: Narrow inline namespace filtration for unqualified friend declarations

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2277-2278 +if (isFriend && !QualifierLoc && !FunctionTemplate) { + SemaRef.FilterLookupForScope(Previous, DC, /*Scope*/ nullptr, + /*ConsiderLinkage

[PATCH] D135366: [clang][Interp] Implement String- and CharacterLiterals

shafik added inline comments. Comment at: clang/test/AST/Interp/arrays.cpp:128 + + static_assert("foobar"[2] == 'o', ""); + Also `2["foobar"]` Comment at: clang/test/AST/Interp/arrays.cpp:135 + static_assert(u32[1] == U'b', ""); +}; + ---

[PATCH] D135433: [clang][Interp] Implement while and do-while loops

shafik added inline comments. Comment at: clang/test/AST/Interp/loops.cpp:9 + +namespace WhileLoop { + constexpr int f() { infinite loop w/o a side effect are undefined behavior and so should be ill-formed and generate a diagnostic e.g. `while(1);`, so we shoul

[PATCH] D135433: [clang][Interp] Implement while and do-while loops

shafik added inline comments. Comment at: clang/test/AST/Interp/loops.cpp:9 + +namespace WhileLoop { + constexpr int f() { tbaeder wrote: > shafik wrote: > > infinite loop w/o a side effect are undefined behavior and so should be > > ill-formed and generate a d

[PATCH] D135551: [clang] replace `assert(0)` with `llvm_unreachable` NFC

shafik added a comment. In D135551#3853365 , @rnk wrote: > I think the status quo has real problems. We pretend that we can do both of > these: > > - Assert liberally, with the understanding that assertion failures lead to UB > (failed bad cast check, b

[PATCH] D135816: [clang] Support `constexpr` for some `ASTNodeKind` member functions

shafik added a comment. LGTM, looks mostly straight forward but I am not super familiar with this code so I will let Aaron also review it too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135816/new/ https://reviews.llvm.org/D135816

[PATCH] D135818: [clang] Update ASM goto documentation to reflect how Clang differs from GCC

shafik added a comment. Thank you for updating the documents! Comment at: clang/docs/LanguageExtensions.rst:1555 + on the indirect branch may result in undefined behavior and should be + avoided. It would be helpful to provide an example of what is not supp

[PATCH] D135818: [clang] Update ASM goto documentation to reflect how Clang differs from GCC

shafik added inline comments. Comment at: clang/docs/LanguageExtensions.rst:1555 + on the indirect branch may result in undefined behavior and should be + avoided. void wrote: > shafik wrote: > > It would be helpful to provide an example of what is not suppor

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:6785 +// other than to declare it, is ill-formed. [...] +//if (CheckReferencedFunctionTrailingRequiresClause(FD, ConstraintFailLoc)) +// return ExprError(); This code is commente

[PATCH] D135013: [clang][Interp] Array initialization via ImplicitValueInitExpr

shafik added a comment. The following also generates an `ImplicitValueInitExpr`: template struct B { T a[N]; }; int main() { constexpr B arr = {}; constexpr int x = arr.a[0]; } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D135013: [clang][Interp] Array initialization via ImplicitValueInitExpr

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:737 +auto ArrayType = IVIE->getType()->getAsArrayTypeUnsafe(); +const auto *CAT = dyn_cast(ArrayType); +const size_t NumElems = CAT->getSize().getZExtValue(); I believe

[PATCH] D135858: [clang][Interp] Support pointer arithmethic in binary operators

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:241 + // Pointer arithmethic special case. This is supported for one of + // LHS and RHS being a pointer type and the other being an integer type. + if (BO->getType()->isPointerType()) { ---

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

shafik added a comment. Thank you for this fix, I have some comments. Comment at: clang/lib/Sema/SemaChecking.cpp:16068 DiagRuntimeBehavior(BaseExpr->getBeginLoc(), BaseExpr, PDiag(DiagID) << toString(index, 10, true) -

[PATCH] D133753: [clang][Interp] WIP: Array fillers

shafik added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.h:83 + /// that have an array filler. + const InterpSize NumFullElems = 0; + const Expr *ArrayFiller = nullptr; How about `NumElemsWithFiller`? Comment at: clang/li

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

shafik added a comment. I commented on the bug report: https://github.com/llvm/llvm-project/issues/54158 I wanted to also add a note here, I don't believe this is a valid change. The example should be ill-formed as was clarified by defect report 2374

[PATCH] D120862: Sema: Allow scoped enums as source type for integral conversion.

shafik added a subscriber: aaron.ballman. shafik added a comment. In D120862#3857340 , @pcc wrote: > Does that DR apply retroactively to C++17? I get the impression that "Status: > C++20" means that the issue was only fixed in C++20, which would make thi

[PATCH] D135013: [clang][Interp] Array initialization via ImplicitValueInitExpr

shafik added a comment. I found a way to get `ImplictValueInitExpr` for a record: https://godbolt.org/z/Eqf9Gz1G4 I am not sure if we can achieve the same in C++ b/c value init of a class break down to default init or zero init. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135013/new

[PATCH] D136036: [Clang] Add __has_constexpr_builtin support

shafik added a comment. In D136036#3860945 , @Izaron wrote: > This macro will be needed because we are making constexpr `` and > ``. We will conditionally make the functions constexpr until all > supported compilers have full support. > > We had a discu

[PATCH] D136036: [Clang] Add __has_constexpr_builtin support

shafik added a comment. The changes to the various `Vist*CallExpr` functions don't seem directly related to supporting `__has_constexpr_builtin`, can you explain the purpose of those changes? If they are not directly related maybe it makes more sense for them to be split off into a follow-up PR

[PATCH] D136017: [clang][Interp] Materializing primitive temporaries

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:476 + + assert(E->getNumObjects() == 0 && "TODO: Implement cleanups"); + return this->visit(SubExpr); What cases do we have multiple objects? Can we add failing tests for those?

[PATCH] D136018: [Clang] Fix crash when checking misaligned member with dependent type

shafik added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:17398-17399 + const bool IsDiscardMisalignedPointer = + T->isPointerType() && + (T->getPointeeType()->isIncompleteType() || T->isDependentType() || + Context.getTypeAlignInC

[PATCH] D135920: [clang][Sema] Use correct array size for diagnostic

shafik requested changes to this revision. shafik added a comment. This revision now requires changes to proceed. The current approach of mixing bytes and indices in the same diagnostic is too confusing. I think we see some acceptable messages for out of bounds access by looking at three differe

[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 { public

[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 cfe-commits@lists.llvm.

[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 clang/test/SemaCXX/anonymous-struc

[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() || Field->isAnonymousStru

[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. Repos

[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 https://lists.llvm.org/cgi-bin/mail

[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 ro

[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 cfe-commits@l

[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 https://reviews.llvm.o

[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 seem

[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] D140547: Perform access checking to private members in simple requirement.

shafik added a comment. Thank you for the fix, can you elaborate on the motivation for this change, it looks like setting `setNamingClass` allows for improved diagnostics but I would like it explained better in the PR description. Comment at: clang/lib/Sema/SemaTemplateInstan

[PATCH] D140554: [C++20] Determine the dependency of unevaluated lambdas more accurately

shafik added a comment. I am not sure if this is moving in the right direction or not, I will wait for @cor3ntin to chime in. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140554/new/ https://reviews.llvm.org/D140554 _

[PATCH] D140587: [clang] Fix a clang crash on invalid code in C++20 mode.

shafik added a comment. Thank you for the fix, can you explain the failure case in more detail and why checking that `RD` is defined is the correct fix. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140587/new/ https://reviews.llvm.org/D140587 __

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

shafik created this revision. shafik added reviewers: rsmith, aaron.ballman, erichkeane. Herald added a project: All. shafik requested review of this revision. Currently in `Sema::getDestructorName` we call `SS.getScopeRep()->getPrefix()` but `SS.getScopeRep()` can return `nullptr` because `Look

[PATCH] D141192: [Clang] Add warnings on bad shifts inside enums.

shafik added a comment. So it looks like in `handleIntIntBinOp` we do hit this code: unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1); if (SA != RHS) { Info.CCEDiag(E, diag::note_constexpr_large_shift) << RHS << E->getType() << LHS.getBitWidth(); So maybe we sho

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

shafik added a comment. Thank you for this diagnostic fix, please make sure you update `clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp`. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:5956 def err_initializer_string_for_char_array_too_long : Error< - "

[PATCH] D72103: [Sema] Avoid using an invalid InsertPos

shafik added a comment. Herald added a project: All. It looks like the original bug no longer reproduces: https://github.com/llvm/llvm-project/issues/42566 Is this PR still needed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D72103/new/ https://reviews.llvm.org/D72103 __

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

shafik added a comment. In D141283#4038703 , @tbaeder wrote: > When grepping `clang/test/` for "for char array is too long", I get a bunch > of other hits for this diagnostic, are the those tests not failing for you > (e.g. `clang/test/Sema/array-init.c

[PATCH] D140809: [clang][Interp] Implement logical and/or operators

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:387 + // Logical AND. + // Visit LHS. Only visit RHS is LHS was TRUE. + LabelTy LabelFalse = this->getLabel(); Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D141414: [clang] add warning on shifting boolean type

shafik added a subscriber: aaron.ballman. shafik added a comment. Hey @aaron.ballman I made some suggestions in the bug report: https://github.com/llvm/llvm-project/issues/28334 do you think this is a reasonable approach? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D141283: [clang] Improve diagnostic for "initializer-string for char array is too long"

shafik accepted this revision. shafik added a comment. LGTM Comment at: clang/test/CXX/dcl.decl/dcl.init/dcl.init.string/p2.cpp:2 // RUN: %clang_cc1 -fsyntax-only -verify %s -char test1[1]="f"; // expected-error {{initializer-string for char array is too long}} +char test1[1]

[PATCH] D136554: Implement CWG2631

shafik added inline comments. Comment at: clang/include/clang/Sema/Sema.h:9602-9604 +return Ctx.Context == + ExpressionEvaluationContext::PotentiallyEvaluatedIfUsed || + Ctx.IsCheckingDefaultArgumentOrInitializer; aaron.ballman wrote:

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:282 + bool isGlobalDecl(const VarDecl *VD) const { +return !VD->hasLocalStorage() || VD->isConstexpr(); + } tbaeder wrote: > shafik wrote: > > tbaeder wrote: > > > shafik wrote

[PATCH] D137491: [clang][NFC] Use c++17 style variable type traits

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137491/new/ https://reviews.llvm.org/D137491 ___

[PATCH] D137545: [clang][Interp] DerivedToBase casts

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137545/new/ https://reviews.llvm.org/D137545 ___

[PATCH] D137558: [clang][Interp] Reject reinterpret_casts

shafik added a comment. In D137558#3915350 , @aaron.ballman wrote: > This is lacking test coverage. (Other interesting related test coverage are > casts that perform a reinterpret_cast under the hood but that might be > somewhat separate.) Well I thin

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

shafik added a comment. I am in the committee meeting this week but if you ping next week I should have time. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D129531/new/ https://reviews.llvm.org/D129531 _

[PATCH] D90568: [clang] Add [is|set]Nested methods to NamespaceDecl

shafik added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:8767-8770 const_cast(*Context), Context->getTranslationUnitDecl(), /*Inline*/ false, SourceLocation(), SourceLocation(), &Context->Idents.get("std"), +/*PrevDecl*/ nullptr, /

[PATCH] D137531: [clang] Add the check of decltype in derived templates for issue #58674

shafik added inline comments. Comment at: clang/docs/ReleaseNotes.rst:787 operator. -- ``clang_Cursor_getNumTemplateArguments``, ``clang_Cursor_getTemplateArgumentKind``, - ``clang_Cursor_getTemplateArgumentType``, ``clang_Cursor_getTemplateArgumentValue`` and +- ``clang_

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

shafik added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:18251-18252 + SemaRef.tryCaptureVariable(V, Loc, Sema::TryCapture_Implicit, + /*EllipsisLoc*/ SourceLocation(), + /*BuildAndDiagnose*/ true, CaptureType,

[PATCH] D137712: Correctly handle Substitution failure in concept specialization.

shafik added inline comments. Comment at: clang/lib/AST/ExprConcepts.cpp:112-113 + return ConceptSpecializationExpr::Create( + C, NNS, TemplateKWLoc, ConceptNameInfo, FoundDecl, NamedConcept, nullptr, + nullptr, Satisfaction); +} I think I got the para

[PATCH] D137720: Migrate getOrCreateInternalVariable from Clang to OMPIRBuilder.

shafik added inline comments. Comment at: llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp:3980 +Elem.second = new GlobalVariable( +M, Ty, /*IsConstant*/ false, GlobalValue::CommonLinkage, +Constant::getNullValue(Ty), Elem.first(), Repository: r

[PATCH] D137751: Produce a more informative diagnostics when Clang runs out of source locations

shafik added inline comments. Comment at: clang/lib/Basic/SourceManager.cpp:2251 + uint64_t CountedSize = 0; + for (int IDIndex = -(int)LoadedSLocEntryTable.size() - 1; + IDIndex < (int)LocalSLocEntryTable.size(); ++IDIndex) { alexfh wrote: > Could you ad

[PATCH] D137570: [Clang][Sema] Refactor category declaration under CheckForIncompatibleAttributes. NFC

shafik added inline comments. Comment at: clang/lib/Sema/SemaStmtAttr.cpp:319 + }; + // There are 7 categories of loop hints attributes: vectorize, interleave, + // unroll, unroll_and_jam, pipeline, distribute, and vectorize predicate. Can you break up this co

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

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane. Herald added a project: All. shafik requested review of this revision. Currently `Sema::CheckForIntOverflow` misses several case that other compilers diagnose for overflow in integral constant expressions. This inclu

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

shafik updated this revision to Diff 474981. shafik added a comment. Fix formatting of test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137897/new/ https://reviews.llvm.org/D137897 Files: clang/lib/Sema/SemaChecking.cpp clang/test/Sema/integer-overflow.cpp Index: clang/test/Sem

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

shafik added a comment. It looks like `conditional_callbacks.c` has been failing every once and while before this change. I can't seem to track down when it started yet. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137897/new/ https://reviews.llvm.org/D137897

[PATCH] D137562: [clang][Interp] Start supporting virtual base classes

shafik added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.cpp:126 + auto CtorSub = [=](unsigned SubOff, Descriptor *F, bool IsBase, + bool Recurse = true) { auto *Desc = reinterpret_cast(Ptr + SubOff) - 1; To recurse or n

[PATCH] D137563: [clang][Interp] Check declarations for constexpr-ness in Load() instructions

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.cpp:101 +static bool CheckConstexpr(InterpState &S, CodePtr OpPC, const Pointer &Ptr) { + if (!S.inConstantContext()) Is `CheckConstexpr` descriptive enough? Would something like `CheckLoadI

[PATCH] D137244: [Clang] Correctly capture bindings in dependent lambdas.

shafik added a comment. This looks reasonable to me but not confident enough to approve it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137244/new/ https://reviews.llvm.org/D137244 ___ cfe-commits mail

[PATCH] D137768: [opt][clang] Enable using -module-summary /-flto=thin with -S / -emit-llvm

shafik added inline comments. Comment at: clang/lib/CodeGen/BackendUtil.cpp:996 +MPM.addPass(PrintModulePass(*OS, "", CodeGenOpts.EmitLLVMUseLists, +true /* EmitLTOSummary */)); + } Nitpick, but we should be consis

[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 clang/test/Sema/integer-overflow.cp

[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: https://reviews.llvm.org/D137897?vs=475528&

[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] 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] 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, Res

[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 `CXXMethodDec

[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 mai

[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 cfe-commits@l

[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 `GH`

[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. Rep

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