[PATCH] D105354: [clang][AST] Add support for DecompositionDecl to ASTImporter.

shafik added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:2305-2309 + BindingDecl *ToD; + if (GetImportedOrCreateDecl(ToD, D, Importer.getToContext(), DC, Loc, + Name.getAsIdentifierInfo())) +return ToD; + martong

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik updated this revision to Diff 357578. shafik added reviewers: jingham, jasonmolenda. shafik added a comment. Changing approach based on Adrian's comments. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://reviews.llvm.org/D105564 Files: lldb/source/Plugins/Expr

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik added a comment. @aprantl after your comments and discussion offline I changed my approach to do this lookup using the symbol table and it worked out. The main issue with the first approach was that gcc would also have to be updated in order for them to change their approach to generatin

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik added a comment. In D105564#2867717 , @probinson wrote: >> Currently when we have a member function that has an auto return type and >> the definition is out of line we generate two DWARF DIE. >> One for the declaration and a second one for the de

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik updated this revision to Diff 358373. shafik added a comment. - Modified `FindTypeForAutoReturnForDIE` to take into account if we have multiple symbols with the same name. - Modified `ParseSubroutine` to take into account that case we get the definition first, this can happen when we set

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:406 + virtual lldb::TypeSP + FindTypeForAutoReturnForDIE(const DWARFDIE &die, teemperor wrote: > If this is virtual then I guess `SymbolFileDWARFDwo` should overl

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik updated this revision to Diff 358773. shafik marked 8 inline comments as done. shafik added a comment. - Removed virtual from `FindTypeForAutoReturnForDIE` - Added missing nullptr checks - Modernized the code CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105564/new/ https://revie

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

shafik added inline comments. Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:402-403 case DW_AT_type: - type = form_value; + if (!type.IsValid()) +type = form_value; break; JDevlieghere wrote: > What's the p

[PATCH] D111477: DO NOT SUBMIT: workaround for context-sensitive use of non-type-template-parameter integer suffixes

shafik added a comment. I think this is fine as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111477/new/ https://reviews.llvm.org/D111477 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

shafik added a comment. How does it handle this case: struct A {int x;}; struct A f(void) { struct A{} a; return a; and this case: struct b{}; struct a {int x;} // visible outside of f f(struct b {int x; } b1) { // not visibile out of f return (stru

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

shafik added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1037 + "void foo(struct Param { int a; } *p);", Lang_C89); + EXPECT_TRUE(testStructuralMatch(Decls)); +} I expect this to be `false` the outer `Param`

[PATCH] D113118: [clang][AST] Check context of record in structural equivalence.

shafik added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:1037 + "void foo(struct Param { int a; } *p);", Lang_C89); + EXPECT_TRUE(testStructuralMatch(Decls)); +} balazske wrote: > shafik wrote: > > I expect

[PATCH] D111509: [clang] use getCommonSugar in an assortment of places

shafik added a comment. Changing how types are printed can effect LLDB expression parsing tests. I ran the LLDB test suite with this change: ninja check-lldb and it changes to the results for `TestScalarURem.py` which can be run using: llvm-lit -sv lldb/test --filter TestScalarURem.py The

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

shafik added a comment. Apologies for the late reply but after the last changes to the diagnostic messages are much better. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135920/new/ https://reviews.llvm.org/D135920 ___

[PATCH] D136036: [Clang] Add __has_constexpr_builtin support

shafik accepted this revision. shafik added a comment. LGTM, thanks for explaining what I was missing. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136036/new/ https://reviews.llvm.org/D136036 ___ cfe-c

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.h:574 +/// 3) Initialized global with index \I with that +template ::T> +bool InitGlobalTemp(InterpState &S, CodePtr OpPC, uint32_t I, tbaeder wrote: > shafik wrote: > > Is `Name` really a `Typ

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

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/D136017/new/ https://reviews.llvm.org/D136017 ___ cfe-commits mailing list cfe-commits@l

[PATCH] D136423: [clang][Interp] Implement inc/dec postfix and prefix operators

shafik accepted this revision. shafik added a comment. LGTM Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1258 + } + case UO_PreDec: { // --x +if (!this->visit(SubExpr)) You could combine this with `UO_PreInc` and just use a `bool` flag to determin

[PATCH] D136012: [clang][Interp] Fix record members of reference type

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:867 - if (Optional T = classify(Init->getType())) { + if (Optional T = classify(Init)) { if (!t

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

shafik added a comment. I am going to do another pass but this is my initial set of comments. Comment at: clang/include/clang-c/Index.h:1537 + * initializer. + * + */ nit: extra line Comment at: clang/include/clang/AST/ExprCXX.h:4728-47

[PATCH] D136457: [clang][Interp] Fix discarding non-primitive function call return values

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1158 + +if (DiscardResult && !T) { + // If we need to discard the return value but the function returns its Could you alternatively use `Func->hasRVO()`? Repository:

[PATCH] D136532: [clang][Interp] Implement left and right shifts

shafik added inline comments. Comment at: clang/test/AST/Interp/shifts.cpp:57 +//c >>= 99; // expected-warning {{shift count >= width of type}} +//c <<= CHAR_BIT; // expected-warning {{shift count >= width of type}} +//c >>= CHAR_BIT; // expected-warning {{shift c

[PATCH] D136532: [clang][Interp] Implement left and right shifts

shafik added inline comments. Comment at: clang/test/AST/Interp/shifts.cpp:70 +i = 1 << (WORD_BIT - 1); // cxx17-warning-not {{sets the sign bit of the shift expression}} +i = -1 << (WORD_BIT - 1); // cxx17-warning {{shifting a negative signed value is undefined}} \ +

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:259 + + if (Op != BO_Add && Op != BO_Sub) +return false; or neither left or right operand is a pointer type Comment at: clang/lib/AST/Interp/ByteCodeExprGen

[PATCH] D136532: [clang][Interp] Implement left and right shifts

shafik added inline comments. Comment at: clang/test/AST/Interp/shifts.cpp:57 +//c >>= 99; // expected-warning {{shift count >= width of type}} +//c <<= CHAR_BIT; // expected-warning {{shift count >= width of type}} +//c >>= CHAR_BIT; // expected-warning {{shift c

[PATCH] D136532: [clang][Interp] Implement left and right shifts

shafik added inline comments. Comment at: clang/test/AST/Interp/shifts.cpp:57 +//c >>= 99; // expected-warning {{shift count >= width of type}} +//c <<= CHAR_BIT; // expected-warning {{shift count >= width of type}} +//c >>= CHAR_BIT; // expected-warning {{shift c

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.cpp:1 //===--- InterpState.cpp - Interpreter for the constexpr VM -*- C++ -*-===// // Probably a separate NFC commit Comment at: clang/lib/AST/Interp/Interp.cpp:465 +b

[PATCH] D136839: [clang][Interp] Handle non-primitive locals without initializer

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/D136839/new/ https://reviews.llvm.org/D136839 ___

[PATCH] D136532: [clang][Interp] Implement left and right shifts

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

[PATCH] D136831: [clang][Interp] Protect Record creation against infinite recusion

shafik accepted this revision. shafik added a comment. LGTM Comment at: clang/test/AST/Interp/records.cpp:144 + // ref-error {{has incomplete type 'const Bar'}} +}; +constexpr Bar B; // expected-error {{must be initialized by a constant expression}} \ ---

[PATCH] D135858: [clang][Interp] Support pointer arithmetic 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] D135858: [clang][Interp] Support pointer arithmetic in binary operators

shafik added inline comments. Comment at: clang/test/AST/Interp/arrays.cpp:69 +constexpr int const * ap1 = &arr[0]; +constexpr int const * ap2 = ap1 + 3; // expected-error {{must be initialized by a constant expression}} \ + // expected-note {

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.h:970 + if (!Pointer::hasSameArray(LHS, RHS)) { +// TODO: Diagnose. +return false; tbaeder wrote: > This is also not being diagnosed (only rejected) by the current interpreter. > But

[PATCH] D136826: [clang][Interp] Make sure we free() allocated InitMaps

shafik added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.cpp:46 + + Ptr += sizeof(InitMap *); for (unsigned I = 0, NE = D->getNumElems(); I < NE; ++I) { I believe `Ptr` is not longer valid b/c of `free(IM)` b/c what `Ptr` points to has not b

[PATCH] D136826: [clang][Interp] Make sure we free() allocated InitMaps

shafik added inline comments. Comment at: clang/lib/AST/Interp/Descriptor.cpp:46 + + Ptr += sizeof(InitMap *); for (unsigned I = 0, NE = D->getNumElems(); I < NE; ++I) { aaron.ballman wrote: > shafik wrote: > > I believe `Ptr` is not longer valid b/c of `free

[PATCH] D136956: [clang][Interp] Implement BitXor opcode

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. Besides the minor comment LGTM Comment at: clang/test/AST/Interp/literals.cpp:290 + static_assert((0 | gimme(12)) == 12, ""); + static_assert((12 ^ true) == 13, ""); +#prag

[PATCH] D135858: [clang][Interp] Support pointer arithmetic in binary 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/D135858/new/ https://reviews.llvm.org/D135858 ___ cfe-commits mailing list cfe-commits@l

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:803 + // Make sure we don't accidentally register the same decl twice. + if (const Decl *VD = Src.dyn_cast(); VD && isa(VD)) { +assert(!P.getGlobal(cast(VD))); Nitpick, I fi

[PATCH] D137020: [clang][AST] Handle variable declaration with unknown typedef in C

shafik added subscribers: aaron.ballman, shafik. shafik added a comment. CC @aaron.ballman Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137020/new/ https://reviews.llvm.org/D137020 ___ cfe-commits maili

[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: > > Why not `hasGlobalStorage()`? > >

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

shafik created this revision. shafik added reviewers: erichkeane, aaron.ballman, thakis. Herald added a project: All. shafik requested review of this revision. In D131528 using `Info.EvalMode == EvalInfo::EM_ConstantExpression` is not strict enough to restrict t

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

shafik added a comment. In D131528#3715841 , @thakis wrote: > We're also still seeing the diag fire after this: > https://ci.chromium.org/p/chromium/builders/ci/ToTLinux > > (And we cleaned up our codebase back when it was still an error.) > > Our bots h

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

This revision was automatically updated to reflect the committed changes. Closed by commit rG2608f553b8fd: [Clang] Tighten restrictions on enum out of range diagnostic (authored by shafik). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://r

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

shafik added a comment. Ok it looks like this is a bit more subtle, I broke the llvm-test-suite file `paq8p.cpp` again. We need both conditions to be true `Info.EvalMode == EvalInfo::EM_ConstantExpression && Info.InConstantContext`. We need to be in a context that requires a constant value but

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

shafik added a comment. In D131528#3717141 , @gulfem wrote: > Can somebody please clarify this? Is the following code results in > `undefined` behavior? > > enum E1 {e11=-4, e12=4}; > E1 x2b = static_cast(8); > > `constexpr E1 x2 = static_cast(8)` se

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

shafik added a comment. In D131704#3717774 , @akhuang wrote: > We're seeing this warning in code with global constants, e.g. > > const Enum x = static_cast(-1); > > is this intended? This is constant initialization, so this is indeed expected. Dependi

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

shafik added inline comments. Comment at: clang/lib/Basic/TypeTraits.cpp:64 +#define TYPE_TRAIT_N(Spelling, Name, Key) 0, +#include "clang/Basic/TokenKinds.def" +}; @aaron.ballman do we really have to include this three times? We are defining different macros so

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

shafik added a comment. In D131528#3718405 , @mstorsjo wrote: > In D131528#3716876 , @shafik wrote: > >> In D131528#3715841 , @thakis wrote: >> >>> We're also still seeing

[PATCH] D131704: [Clang] Tighten restrictions on enum out of range diagnostic

shafik added a comment. In D131704#3719954 , @thakis wrote: >> This is constant initialization, so this is indeed expected. > > Do we have a test for this in clang? It seems that the diag appeared for that > case after the change that turned this diag in

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

shafik added a comment. In D131528#3719430 , @ayermolo wrote: > Was this and previous change intended to capture this case? > const enum NumberType neg_one = (enum NumberType) ((enum NumberType) 0 - > (enum NumberType) 1); That was not intended and I p

[PATCH] D131528: [Clang] Restrict non fixed enum to a value outside the range of the enumeration values warning to context requiring a constant expression

shafik added a comment. In D131528#3719561 , @mstorsjo wrote: > In D131528#3719414 , @shafik wrote: > >> In D131528#3718405 , @mstorsjo >> wrote: >> >>> In D131528#371687

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

shafik created this revision. shafik added reviewers: erichkeane, aaron.ballman, thakis, mstorsjo. Herald added a project: All. shafik requested review of this revision. The restrictions added in D131704 were not sufficient to avoid all non-constant expression c

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

shafik updated this revision to Diff 452775. shafik marked an inline comment as done. shafik added a comment. - Changed `NotConstexprVar` to `ConstExprVar` - Moved checking of `ConstexprVar` up so that we do the less expensive check first CHANGES SINCE LAST ACTION https://reviews.llvm.org/D13

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:13547-13551 + if (auto ValD = Info.EvaluatingDecl.dyn_cast()) { +const VarDecl *VD = dyn_cast_or_null(ValD); +if (VD && !VD->isConstexpr()) + NotConstexprVar = true; + } --

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:13547-13551 + if (auto ValD = Info.EvaluatingDecl.dyn_cast()) { +const VarDecl *VD = dyn_cast_or_null(ValD); +if (VD && !VD->isConstexpr()) + NotConstexprVar = true; + } --

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplateDeduction.cpp:746 unsigned Depth, Index; -std::tie(Depth, Index) = getDepthAndIndex(Unexpanded[I]); +std::tie(Depth, Index) = getDepthAndIndex(U); if (Depth == Info.getDeducedDe

[PATCH] D131933: DebugInfo: Remove auto return type representation support

shafik added a comment. If we are not deriving any useful functionality from supporting this in debug-info then this makes sense to me but I will let someone still directly involved to approve it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1319

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

shafik updated this revision to Diff 453062. shafik marked an inline comment as done. shafik added a comment. - Addressing comments on casting of `EvaluatingDecl` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131874/new/ https://reviews.llvm.org/D131874 Files: clang/lib/AST/ExprConst

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

shafik updated this revision to Diff 453350. shafik marked 2 inline comments as done. shafik added a comment. - Simplify condition to set `ConstexprVar` even more CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131874/new/ https://reviews.llvm.org/D131874 Files: clang/lib/AST/ExprConst

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

shafik updated this revision to Diff 453389. shafik marked an inline comment as done. shafik added a comment. - Added `consteval` test with default parameters CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131874/new/ https://reviews.llvm.org/D131874 Files: clang/lib/AST/ExprConstant.

[PATCH] D131874: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant initialization

This revision was automatically updated to reflect the committed changes. Closed by commit rG8de51375f12d: [Clang] Tighten restrictions on enum out of range diagnostic to avoid constant… (authored by shafik). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D132098: [clang][Interp] Implement inv and neg unary operations

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:131 + + return this->emitNE(*T, SubExpr); +} Can you explain this line? Comment at: clang/lib/AST/Interp/Interp.h:183 + + S.Stk.push(Val); + return tr

[PATCH] D132098: [clang][Interp] Implement inv and neg unary operations

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:131 + + return this->emitNE(*T, SubExpr); +} shafik wrote: > Can you explain this line? Nevermind, I get it, you are emitting a `0` and then doing a not equal to zero to

[PATCH] D132111: [clang][Interp] Implement pointer (de)ref operations and DeclRefExprs

shafik added a comment. This mostly makes sense to me but I will let @tahonermann review it as well. Comment at: clang/test/AST/Interp/literals.cpp:41 + +constexpr const int* getIntPointer() { + return &m; What are these functions testing? CHANGES SINCE LAS

[PATCH] D132136: [clang] Perform implicit lvalue-to-rvalue cast with new interpreter

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:14938 + + if (E->getType().isNull()) +return false; Curious why these two checks don't go w/ the `::Evaluate(Result, Info, E)` below. Repository: rG LLVM Github Monorepo CHANGES S

[PATCH] D132286: [clang][Interp] Implement function calls

shafik added inline comments. Comment at: clang/test/AST/Interp/functions.cpp:38 +static_assert(add_second(10, 3, true) == 13, ""); +static_assert(add_second(300, -20, false) == 300, ""); I would expect a lot more test cases: trailing return types, default argume

[PATCH] D132415: [LLDB] Add data formatter for std::coroutine_handle

shafik added inline comments. Comment at: lldb/source/Plugins/Language/CPlusPlus/Coroutines.cpp:39 + CompilerType coro_func_type = ast_ctx.CreateFunctionType( + /*result_type*/ void_type, /*args*/ &void_type, /*num_args*/ 1, + /*is_variadic*/ false, /*qualifiers*/ 0);

[PATCH] D132659: PotentiallyEvaluatedContext in a ImmediateFunctionContext.

shafik added a comment. This looks good to me but I want another set of eyes on this. Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:942 +} // namespace GH51182 \ No newline at end of file Please add a newline Repository: rG LLVM Github Monorepo CHANG

[PATCH] D132659: PotentiallyEvaluatedContext in a ImmediateFunctionContext.

shafik added a subscriber: cor3ntin. shafik added a comment. CC @cor3ntin Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132659/new/ https://reviews.llvm.org/D132659 ___ cfe-commits mailing list cfe-commi

[PATCH] D132695: [Clang] Avoid crashes when parsing using enum declarations

shafik created this revision. shafik added reviewers: erichkeane, aaron.ballman. Herald added a project: All. shafik requested review of this revision. In `Parser::ParseUsingDeclaration(...)` when we call `ParseEnumSpecifier(...)` it is not calling `SetTypeSpecError()` on `DS` when it detects an

[PATCH] D132712: [Clang] Fix assert in Sema::LookupTemplateName so that it does not attempt an unconditional cast to TagType

shafik created this revision. shafik added reviewers: erichkeane, aaron.ballman. Herald added a project: All. shafik requested review of this revision. In `Sema::LookupTemplateName(...)` seeks to assert that the `ObjectType` is complete or being defined. If the type is incomplete it will attempt

[PATCH] D132712: [Clang] Fix assert in Sema::LookupTemplateName so that it does not attempt an unconditional cast to TagType

shafik updated this revision to Diff 455784. shafik added a comment. Apply clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132712/new/ https://reviews.llvm.org/D132712 Files: clang/lib/Sema/SemaTemplate.cpp clang/test/SemaCXX/member-expr.cpp Index: clang/test/SemaCXX/m

[PATCH] D132712: [Clang] Fix assert in Sema::LookupTemplateName so that it does not attempt an unconditional cast to TagType

shafik updated this revision to Diff 455935. shafik marked an inline comment as done. shafik added a comment. - Change to order of operations in assert to put off more costly check - Add release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132712/new/ https://reviews.llvm.org/D132

[PATCH] D132712: [Clang] Fix assert in Sema::LookupTemplateName so that it does not attempt an unconditional cast to TagType

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG21dfe482e13e: [Clang] Fix assert in Sema::LookupTemplateName so that it does not attempt an… (authored by shafik). Herald added a project: clang. Re

[PATCH] D132739: [clang][Interp] Implement IntegralToBoolean casts

shafik added inline comments. Comment at: clang/lib/AST/Interp/Boolean.h:90 + template static Boolean from(T Value) { +if constexpr (std::is_integral::value) + return Boolean(Value != 0); This should work for `floating_point` as well. ===

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:290 +return this->emitConst( +E, Ctx.getASTContext().getTypeSizeInChars(ArgType).getQuantity()); + } aaron.ballman wrote: > erichkeane wrote: > > shafik wrote: > > > I

[PATCH] D134020: [clang][Interp] Handle enums

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:861 + +return this->emitConst(T, getIntWidth(ECD->getType()), ECD->getInitVal(), + E); If I check out `IntExprEvaluator::CheckReferenceDecl(...)` it i

[PATCH] D132990: [Clang] Fix compat diagnostic to detect a nontype template parameter has a placeholder type using getContainedAutoType()

shafik added a comment. ping, CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132990/new/ https://reviews.llvm.org/D132990 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D134057: [clang][Interp] Start implementing record types

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:316 + // Base above gives us a pointer on the stack. + const auto *FD = dyn_cast(Member); + assert(FD); tbaeder wrote: > erichkeane wrote: > > I THINK Member is a ValueDecl beca

[PATCH] D132990: [Clang] Fix compat diagnostic to detect a nontype template parameter has a placeholder type using getContainedAutoType()

shafik updated this revision to Diff 461033. shafik added a comment. - Add release notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132990/new/ https://reviews.llvm.org/D132990 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaTemplate.cpp clang/test/SemaTemplate/gh57362.

[PATCH] D132990: [Clang] Fix compat diagnostic to detect a nontype template parameter has a placeholder type using getContainedAutoType()

shafik updated this revision to Diff 461088. shafik marked an inline comment as done. shafik added a comment. - Update wording in release note. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D132990/new/ https://reviews.llvm.org/D132990 Files: clang/docs/ReleaseNotes.rst clang/lib/Se

[PATCH] D132990: [Clang] Fix compat diagnostic to detect a nontype template parameter has a placeholder type using getContainedAutoType()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf8a37a6ce674: [Clang] Fix compat diagnostic to detect a nontype template parameter has a… (authored by shafik). Herald added a project: clang. Repos

[PATCH] D134136: [Clang] Support constexpr for builtin fmax, fmin, ilogb, logb, scalbn

shafik added inline comments. Comment at: llvm/lib/Support/APFloat.cpp:4206 + else if (Arg.isZero()) +Result.makeInf(/* Negative = */ true); + else if (Arg.isInfinity()) This is the format as documented by clang-tidy bugprone argument comment see here: ht

[PATCH] D134286: [C2x] implement typeof and typeof_unqual

shafik added inline comments. Comment at: clang/include/clang/AST/ASTContext.h:1717 /// GCC extension. + QualType getTypeOfExprType(Expr *E, bool IsUnqual) const; I guess these are not purely extensions anymore? Comment at: clang/include/

[PATCH] D134231: [clang][C++20] p0960 Allow initializing aggregates from a parenthesized list of values

shafik added a comment. Hello Ed, thank you for picking up this work. I liked the approach in the first PR, did you consider reaching out to the author is asking if it was ok to commandeer the work to allow it to move forward? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

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

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane. Herald added a project: All. shafik requested review of this revision. When attempting to decide if in C++17 a type template for class template argument deduction and the code is ill-formed the condition to break is

[PATCH] D133289: [C2X] N3007 Type inference for object definitions

shafik added inline comments. Comment at: clang/test/Sema/c2x-auto.c:49 +auto b = 9; +auto c = a + b; + } aaron.ballman wrote: > to268 wrote: > > shafik wrote: > > > When I made the comment about the example from the proposal, this was > > > what I was

[PATCH] D53847: [C++2a] P0634r3: Down with typename!

shafik added a subscriber: aaron.ballman. shafik added a comment. I made mostly small comments but I think @aaron.ballman and/or @erichkeane should take a look as well. Comment at: clang/lib/Parse/ParseDecl.cpp:3434-3435 &SS) && -

[PATCH] D134020: [clang][Interp] Handle enums

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/AST/Interp/enums.cpp:25 + SIX = FIVE + 2, + +}; Maybe some edge case values for enumerators like `__INT_MAX__ *2U +1U` (UINT_M

[PATCH] D133574: [C2x] reject type definitions in offsetof

shafik added a comment. LGTM but I will Aaron give the final approval. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133574/new/ https://reviews.llvm.org/D133574 ___ cfe-commits mailing list cfe-commits@

[PATCH] D121532: [Clang] Fix Unevaluated Lambdas

shafik added a comment. @cor3ntin I wonder if these asserts are due to the partial implementation here: https://github.com/llvm/llvm-project/issues/57960 I am happy to look into it if you can point me in the right direction. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION ht

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.h:164 + + if (RHS.isZero()) { +const SourceInfo &Loc = S.Current->getSource(OpPC); You also need to catch when the result is not representable e.g `INT_MIN % -1` see `CheckICE(...)` CHA

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.h:183 + + if (RHS.isZero()) { +const SourceInfo &Loc = S.Current->getSource(OpPC); Just like rem we need to catch `INT_MIN / -1` `CheckICE(...)` looks like it does checking for both in th

[PATCH] D134361: [clang][Interp] Fix copy constructors of structs with array members

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:649 + +if (!ElemT) + return false; Curious what case requires this check? Comment at: clang/lib/AST/Interp/ByteCodeExprGen.h:382 + ByteCodeExprGen *Ctx

[PATCH] D134804: [clang][Interp] Implement bitwise Not operations

shafik added inline comments. Comment at: clang/test/AST/Interp/literals.cpp:72 +static_assert(~-1 == 0, ""); +static_assert(~255 == -256, ""); + Some more tests covering `~INT_MIN == INT_MAX` and vice versa and unscoped enum case as well e.g. ``` enum E {}; E

[PATCH] D134804: [clang][Interp] Implement bitwise Not operations

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

[PATCH] D134801: [clang][Interp] Implement ConditionalOperators

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:357 + + if (!this->visit(Condition)) +return false; Maybe I am misunderstanding what this is doing but can't we just check the result of the condition and then just visit eit

[PATCH] D134801: [clang][Interp] Implement ConditionalOperators

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:357 + + if (!this->visit(Condition)) +return false; tbaeder wrote: > shafik wrote: > > Maybe I am misunderstanding what this is doing but can't we just check the > > result o

[PATCH] D134874: [Concepts] Fix Concepts on generic lambda in a VarTemplateSpecDecl

shafik added a comment. I made mostly minor comments, the diff is difficult to follow wrt to what has really changed but it mostly makes sense. I will probably need a second look. Comment at: clang/lib/Sema/SemaConcept.cpp:508-510 MLTAL = getTemplateInstantiationArgs(FD, nu

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

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LFTM besides the comment about the test. Comment at: clang/test/SemaCXX/specialization-diagnose-crash.cpp:2 +// RUN: %clang_cc1 -fsyntax-only %s -verify +// expected-no-diagn

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