[PATCH] D146329: [Clang] Fix defaulted equality operator so that it does not attempt to compare unnamed bit-fields

shafik updated this revision to Diff 512983. shafik added a comment. - Added codegen test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146329/new/ https://reviews.llvm.org/D146329 Files: clang/lib/Sema/SemaDeclCXX.cpp clang/test/CXX/class/class.compare/class.compare.default/p1.cpp

[PATCH] D146329: [Clang] Fix defaulted equality operator so that it does not attempt to compare unnamed bit-fields

shafik added a comment. @erichkeane let me know if the test I added is sufficient CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146329/new/ https://reviews.llvm.org/D146329 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists

[PATCH] D147920: [clang] Add test for CWG399

shafik added a subscriber: rsmith. shafik added inline comments. Comment at: clang/test/CXX/drs/dr3xx.cpp:1439 + +namespace dr399 { // dr399: 11 + // NB: reuse dr244 test Endill wrote: > shafik wrote: > > Endill wrote: > > > Despite a couple of

[PATCH] D146329: [Clang] Fix defaulted equality operator so that it does not attempt to compare unnamed bit-fields

shafik updated this revision to Diff 513345. shafik marked an inline comment as done. shafik added a comment. - Update codegen test based on feedback CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146329/new/ https://reviews.llvm.org/D146329 Files: clang/lib/Sema/SemaDeclCXX.cpp cla

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

shafik updated this revision to Diff 513397. shafik marked 8 inline comments as done. shafik added a comment. - Updating diagnostic wording, formatting and names - Updated release notes - Updated c++ status - Updates tests to comment that they will change once we fully support P2448R2 - Various mi

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

shafik added inline comments. Comment at: clang/test/CXX/class/class.compare/class.compare.default/p4.cpp:162 + +my_struct obj; // extension-note {{in instantiation of template class 'GH61238::my_struct' requested here}} +} aaron.ballman wrote: > Can you add an

[PATCH] D148206: [clang] Do not crash after suggesting typo correction to constexpr if condition

shafik added a comment. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148206/new/ https://reviews.llvm.org/D148206 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D148260: [clang] Mark CWG2331 as N/A

shafik added a comment. The defect report has two examples even though the first one is commented incorrectly considering the final resolution. I am sure they are covered in the test suite in other places but why not add them? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

shafik updated this revision to Diff 513412. shafik added a comment. Fix DiagnosticSemaKinds.td , was missing an opening quote CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146090/new/ https://reviews.llvm.org/D146090 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Dia

[PATCH] D148372: [clang] add diagnose when member function contains invalid default argument

shafik added a comment. Thank you for the fix! While this will fix the crash I am a little concerned that I don't a diagnostic for each place we call `ActOnParamDefaultArgumentError(...)`, there is only once place where we diagnose this the rest don't. So I wonder if this was purposeful or not

[PATCH] D148274: [clang] Fix overly aggressive lifetime checks for parenthesized aggregate initialization

shafik added inline comments. Comment at: clang/include/clang/Sema/Initialization.h:400 + static InitializedEntity InitializeMemberFromParenAggInit(FieldDecl *Member) { +return InitializedEntity(Member, nullptr, false, false, true); + }

[PATCH] D148274: [clang] Fix overly aggressive lifetime checks for parenthesized aggregate initialization

shafik added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:5332 +if (Arg) + IS.Diagnose(S, SubEntity, SubKind, Arg); +else Do we have tests that trigger this diagnostic and the one right below? Comment at: cl

[PATCH] D146329: [Clang] Fix defaulted equality operator so that it does not attempt to compare unnamed bit-fields

shafik updated this revision to Diff 513754. shafik added a comment. - Add release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146329/new/ https://reviews.llvm.org/D146329 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaDeclCXX.cpp clang/test/CXX/class/class.compare/

[PATCH] D146329: [Clang] Fix defaulted equality operator so that it does not attempt to compare unnamed bit-fields

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6d0fab467efb: [Clang] Fix defaulted equality operator so that it does not attempt to compare… (authored by shafik). Herald added a project: clang. R

[PATCH] D148260: [clang] Mark CWG2331 as N/A

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

[PATCH] D148330: [clang] Do not crash on undefined template partial specialization

shafik added inline comments. Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:134 "specifier in SFINAE context?"); -if (!hasReachableDefinition(PartialSpec)) +if (PartialSpec->hasDefinition() && +!h

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

shafik created this revision. shafik added reviewers: rsmith, aaron.ballman, erichkeane, dim. Herald added a project: All. shafik requested review of this revision. `ResolveConstructorOverload` needs to check properly if we are going to use copy elision we can't use a conversion function. This f

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:743 << FD << Active->InstantiationRange; + } else if (ClassTemplateDecl *CTD = dyn_cast(D)) { +Diags.Report(Active->PointOfInstantiation, I added this f

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

shafik updated this revision to Diff 514091. shafik marked 3 inline comments as done. shafik added a comment. - Fix up A3 test to be less malformed - Fix other test so we confirm we are attempting to call the right contructor CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148474/new/ htt

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplateInstantiate.cpp:746 + diag::note_template_class_instantiation_here) +<< CTD << Active->InstantiationRange; } else { rsmith wrote: > This diagnostic won't i

[PATCH] D148433: [clang] Add tests for DRs about complete-class context

shafik added subscribers: aaron.ballman, rsmith. shafik added inline comments. Comment at: clang/test/CXX/drs/dr18xx.cpp:139 +namespace dr1890 { // dr1890: no drafting +// FIXME: all the examples are well-formed. So this is still in drafting: https://www.open-

[PATCH] D148433: [clang] Add tests for DRs about complete-class context

shafik added inline comments. Comment at: clang/test/CXX/drs/dr16xx.cpp:46 +namespace dr1626 { // dr1626: no open +// FIXME: all of the examples are well-formed +#if __cplusplus >= 201103L Since this is still open, should we be expressing an opinion on whether th

[PATCH] D150946: [clang][Interp] PointerToIntegral 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/D150946/new/ https://reviews.llvm.org/D150946 ___

[PATCH] D150948: [clang][RecoveryExpr] Fix a crash where a dependent type crahes on c-only code path.

shafik accepted this revision. shafik added a comment. Thank you for the fix, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150948/new/ https://reviews.llvm.org/D150948 ___ cfe-commits mailing list

[PATCH] D150435: [clang] Fix crash on attempt to initialize union with flexible array member

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM after addressing Aaron's comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150435/new/ https://reviews.llvm.org/D150435 __

[PATCH] D151032: [clang] Add test for CWG2213

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. Thank you, I think more examples are never bad, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151032/new/ https://reviews.llvm.org/D151032

[PATCH] D151034: [clang] Add test for CWG1397

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

[PATCH] D151034: [clang] Add test for CWG1397

shafik added inline comments. Comment at: clang/test/CXX/drs/dr13xx.cpp:488 +#if __cplusplus == 201103L + // expected-error@#dr1397-struct-A {{default member initializer for 'p' needed within definition of enclosing class 'A' outside of member functions}} + // expected-note@#d

[PATCH] D151042: [clang] Add tests for CWG issues 977, 1482, 2516

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

[PATCH] D151200: [Clang][C++20] Error out if parameter types of a defaulted comparison operator aren't as expected.

shafik accepted this revision. shafik added a comment. LGTM assuming precommit CI comes back green Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151200/new/ https://reviews.llvm.org/D151200 ___ cfe-commi

[PATCH] D151235: [Clang] Switch from TransformExpr to TransformInitializer in places we need to revert initializer to it syntactic form for Sema

shafik created this revision. shafik added reviewers: rsmith, aaron.ballman, erichkeane. Herald added a project: All. shafik requested review of this revision. In some cases we are using `TransformExpr` instead of `TransformInitializer`, this results in `ExprWithCleanups` being dropped and we are

[PATCH] D151235: [Clang] Switch from TransformExpr to TransformInitializer in places we need to revert initializer to it syntactic form for Sema

shafik added a subscriber: NoQ. shafik added inline comments. Comment at: clang/test/Analysis/missing-bind-temporary.cpp:10 namespace variant_0 { -// This variant of the code works correctly. Function foo() is not a template -// function. Note that there are two destructors with

[PATCH] D151214: [clang][Sema] `-Wshadow` warns about shadowings by static local variables

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/SemaCXX/warn-shadow.cpp:33 void foo() { int i; // expected-warning {{declaration shadows a variable in namespace '(anonymous)'}} int j; //

[PATCH] D151094: [clang] Implement P2564 "consteval must propagate up"

shafik added inline comments. Comment at: clang/lib/Parse/ParseDecl.cpp:27 #include "clang/Sema/ParsedTemplate.h" -#include "clang/Sema/Scope.h" #include "clang/Sema/SemaDiagnostic.h" This looks like an unrelated change after removing the change below. Reposi

[PATCH] D151235: [Clang] Switch from TransformExpr to TransformInitializer in places we need to revert initializer to it syntactic form for Sema

shafik updated this revision to Diff 525248. shafik added a comment. - Update test gh62818.cpp to be more precise CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151235/new/ https://reviews.llvm.org/D151235 Files: clang/lib/Sema/TreeTransform.h clang/test/Analysis/missing-bind-tempor

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

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

[PATCH] D151365: [Sema] cast to CXXRecoradDecl correctly when diag a default comparison method

shafik added a comment. Comments here are relavant: https://github.com/llvm/llvm-project/issues/62102 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151365/new/ https://reviews.llvm.org/D151365 ___ cfe-co

[PATCH] D151235: [Clang] Switch from TransformExpr to TransformInitializer in places we need to revert initializer to it syntactic form for Sema

shafik updated this revision to Diff 525385. shafik added a comment. - Update test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151235/new/ https://reviews.llvm.org/D151235 Files: clang/lib/Sema/TreeTransform.h clang/test/Analysis/missing-bind-temporary.cpp clang/test/CodeGenCXX

[PATCH] D151235: [Clang] Switch from TransformExpr to TransformInitializer in places we need to revert initializer to it syntactic form for Sema

shafik marked 2 inline comments as done. shafik added inline comments. Comment at: clang/test/CodeGenCXX/gh62818.cpp:1 +// RUN: %clang_cc1 -no-opaque-pointers -std=c++17 -emit-llvm -triple x86_64-linux-gnu -o - %s | FileCheck %s + Fznamznon wrote: > Why no opaqu

[PATCH] D151235: [Clang] Switch from TransformExpr to TransformInitializer in places we need to revert initializer to it syntactic form for Sema

This revision was automatically updated to reflect the committed changes. shafik marked an inline comment as done. Closed by commit rG2a23de01e515: [Clang] Switch from TransformExpr to TransformInitializer in places we need to… (authored by shafik). Herald added a project: clang. Repository: rG

[PATCH] D151515: [CodeGen] add additional cast when checking call arguments

shafik added a comment. Thank you for the patch! Please apply clang-format at least one of pre-commit failures is due to that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151515/new/ https://reviews.llvm.org/D151515

[PATCH] D147307: [clang] Do not require GNUInlineAttr for inline builtins

shafik added a comment. This PR seems related to this crash: https://github.com/llvm/llvm-project/issues/62958 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147307/new/ https://reviews.llvm.org/D147307

[PATCH] D151523: [ASTStructuralEquivalence] Fix crash when ObjCCategoryDecl doesn't have corresponding ObjCInterfaceDecl.

shafik added inline comments. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:2062 + *Intf2 = D2->getClassInterface(); + if ((Intf1 != nullptr) != (Intf2 != nullptr)) +return false; I think this would be easier to read if you

[PATCH] D151763: [clang] Fix crash when passing a braced-init list to a parentehsized aggregate init expression

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. Thank you for the quick fix! LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151763/new/ https://reviews.llvm.org/D151763 __

[PATCH] D151634: [clang] Add test for CWG253

shafik added inline comments. Comment at: clang/test/CXX/drs/dr0xx.cpp:1022 -namespace dr78 { // dr78: sup +namespace dr78 { // dr78: no // Under DR78, this is valid, because 'k' has static storage duration, so is This is [issue 1380](https://github.com

[PATCH] D151704: [clang] Add test for CWG873

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

[PATCH] D151634: [clang] Add test for CWG253

shafik added a comment. In D151634#4383428 , @Endill wrote: > @shafik Thank you for letting me know! Now I wonder if it was possible to > realize by myself without prior knowledge of CWG 2536, because I feel like > some cross-references are missing. > C

[PATCH] D151753: [Clang][Sema] Do not try to analyze dependent alignment during -Wcast-align

shafik added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:16163 +// Dependent alignment cannot be resolved -> bail out. +if (any_of(VD->specific_attrs(), + [](auto *A) { return A->isAlignmentDependent(); })) Can't

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

shafik added a reviewer: clang-language-wg. shafik added a comment. Adding more reviewers for more visibility. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134334/new/ https://reviews.llvm.org/D134334 ___ cfe-commits mailing list cfe-commits

[PATCH] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

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

[PATCH] D151634: [clang] Add test for CWG253

shafik added inline comments. Comment at: clang/test/CXX/drs/dr0xx.cpp:1022 -namespace dr78 { // dr78: sup +namespace dr78 { // dr78: no // Under DR78, this is valid, because 'k' has static storage duration, so is shafik wrote: > This is [issue 1380](ht

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

shafik updated this revision to Diff 527937. shafik added a comment. - Update check to use tok::annot_cxxscope CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134334/new/ https://reviews.llvm.org/D134334 Files: clang/docs/ReleaseNotes.rst clang/lib/Parse/ParseTentative.cpp clang/te

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

shafik marked an inline comment as done. shafik added inline comments. Comment at: clang/lib/Parse/ParseTentative.cpp:1553-1554 return TPResult::Error; - if (Tok.isNot(tok::identifier)) + if (NextToken().isNot(tok::identifier))

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

shafik added a comment. `clang/test/ClangScanDeps/modules-full.cpp ` passes locally for me and I don't see why my change would effect this test. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134334/new/ https://reviews.llvm.org/D134334 ___ c

[PATCH] D152009: [clang] Fix assertion while parsing an invalid for loop

shafik added inline comments. Comment at: clang/lib/Parse/ParseStmt.cpp:2200 Diag(Tok, diag::err_expected_semi_for); - else -// Skip until semicolon or rparen, don't consume it. Can you explain why you removed this? Commen

[PATCH] D143142: [clang][lex] Enable Lexer to grow its buffer

shafik added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:1203 if (L && !L->isLexingRawMode()) - L->Diag(CP-2, diag::trigraph_ignored); + L->Diag(CP - 2 - L->getBuffer().data(), diag::trigraph_ignored); return 0; I wonder do we really

[PATCH] D143142: [clang][lex] Enable Lexer to grow its buffer

shafik added reviewers: aaron.ballman, cor3ntin, tahonermann. shafik added a comment. WG21 is meeting all this week, so a bunch of folks who should take a look at this may not get around to it right away. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.or

[PATCH] D137113: [Clang] refactor CodeGenFunction::EmitAsmStmt NFC

shafik added inline comments. Comment at: clang/lib/CodeGen/CGStmt.cpp:2405 + QualType Ty = + CGF.getContext().getIntTypeForBitwidth(Size, /*Signed*/ false); + if (Ty.isNull()) { To be consistent with [bugprone-argument-comment](https://clang.

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

shafik added a comment. In D142867#4108079 , @eaeltsin wrote: > The warning now fires even if overflow is prevented with `if constexpr`: > > if constexpr (width <= 64) { > if constexpr (width == 64) { > return 1; > } > return -static_c

[PATCH] D143891: [Clang] Adjust triviality computation in QualType::isTrivialType to C++20 cases.

shafik added a comment. In D143891#4122731 , @aaron.ballman wrote: > In D143891#4122668 , @royjacobson > wrote: > >> In D143891#4122660 , >> @aaron.ballman wrote: >> >>

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

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

[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. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142604/new/ https://reviews.llvm.org/D142604 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144011: [clang]Fix warning for signed conversion

shafik added a comment. If I look at the clang docs for Wconversion I see it includes `-Wshorten-64-to-32` which I believe this is a case of. I think maybe the warning needs a better warning for this case? CHANGES SINCE LAST

[PATCH] D144054: [Lex] Fix a crash in updateConsecutiveMacroArgTokens.

shafik added inline comments. Comment at: clang/lib/Lex/TokenLexer.cpp:1023 Partition = All.take_while([&](const Token &T) { - return T.getLocation() >= BeginLoc && T.getLocation() < Limit && - NearLast(T.getLocation()); + // NOTE: the Limit is included

[PATCH] D144100: [clang] Fix a bug that allowed some overflowing octal escape sequences

shafik added inline comments. Comment at: clang/lib/Lex/LiteralSupport.cpp:266 } - if (ResultChar & 0x02000) + // About to shift out a digit? + if (ResultChar & 0xE000) Maybe the comment should be more specific such as "one of the to

[PATCH] D144192: GH60642: Fix ICE when checking a lambda defined in a concept definition

shafik added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:4572 : Sema::ExpressionEvaluationContext::ConstantEvaluated, -/*LambdaContextDecl=*/nullptr, /*ExprContext=*/ +Sema::ReuseLambdaContextDecl, /*ExprContext=*/ Sema::Ex

[PATCH] D140723: [clang][Interp] Only check constructors for global variables

shafik added a comment. So we are only checking global constructors b/c it is valid in a constant expression context to initialize a record and not initialize all their fields as long as we don't use any of those fields. Note, cases that stem from this has been discussed as part of https://git

[PATCH] D140723: [clang][Interp] Only check constructors for global variables

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

[PATCH] D141194: [clang][Interp] Implement bitcasts

shafik added a comment. I think `VisitCastExpr` is the right place to start looking for what to do. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D141194/new/ https://reviews.llvm.org/D141194 ___ cfe-comm

[PATCH] D141591: [clang][Interp] Properly identify not-yet-defined functions

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1486 bool IsBeingCompiled = Func && !Func->isFullyCompiled(); - bool WasNotDefined = Func && !Func->hasBody(); + bool WasNotDefined = Func && !Func->isConstexpr() && !Func->hasBody(); --

[PATCH] D136751: [clang][Interp] This pointers are writable in constructors

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.cpp:264-270 + const Function *Func = S.Current->getFunction(); + if (Func && Func->isConstructor()) { +// The This pointer is writable in constructors, even if +// isConst() returns true. +if (Ptr

[PATCH] D136751: [clang][Interp] This pointers are writable in constructors

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.cpp:264-270 + const Function *Func = S.Current->getFunction(); + if (Func && Func->isConstructor()) { +// The This pointer is writable in constructors, even if +// isConst() returns true. +if (Ptr

[PATCH] D144285: [Clang] Implement CWG2518 - static_assert(false)

shafik added inline comments. Comment at: clang/docs/ReleaseNotes.rst:62 directly rather than instantiating the definition from the standard library. +- Implemented `CWG2518 `_ which allows ``static_assert(false)`` + not to be ill-formed when its c

[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 498510. shafik added a comment. - Add release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142604/new/ https://reviews.llvm.org/D142604 Files: clang/docs/ReleaseNotes.rst clang/lib/Lex/TokenLexer.cpp clang/test/Preprocessor/macro_vaopt_p

[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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd6d59e660bc7: [Clang] Fix __VA_OPT__ implementation so that it treats the concatenation of a… (authored by shafik). Herald added a project: clang. R

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

shafik added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:1301-1302 SourceLocation AttrNameLoc = ConsumeToken(); -Attr.addNew(AttrName, AttrNameLoc, nullptr, AttrNameLoc, nullptr, 0, -ParsedAttr::AS_Keyword); +Attribut

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

shafik added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:537 +if (!CapturedPattern->isParameterPack()) { + ValueDecl *CapturedVar = + LambdaClass->getCapture(Instantiated)->getCapturedVar(); This is just duplicated across the `if

[PATCH] D124351: [Clang] Implement Change scope of lambda trailing-return-type

shafik added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:19036 bool Sema::tryCaptureVariable( ValueDecl *Var, SourceLocation ExprLoc, TryCaptureKind Kind, We have a bunch of bugs that crash in the method and it would be good to rescreen thos

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

shafik updated this revision to Diff 519555. shafik marked 4 inline comments as done. shafik added a comment. - Removed diagnostic group - fixed cxx_status indentation CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146090/new/ https://reviews.llvm.org/D146090 Files: clang/docs/Release

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb4692f292630: [Clang] Updating handling of defaulted comparison operators to reflect changes… (authored by shafik). Herald added a project: clang. R

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

shafik added a comment. I am about to fix it Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146090/new/ https://reviews.llvm.org/D146090 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lis

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

shafik added a comment. Landed fix 7887af027ee5eff27bbc953074726ab8d9d9f592 There was also 058f04ea7dcbafbeed271fa75ee65e41409b4479 and b323b407f76d22bfc0

[PATCH] D149550: [clang][Interp] Fix compound assign operator evaluation order

shafik added a subscriber: rsmith. shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:683-685 + // C++17 onwards require that we evaluate the RHS first. + // Compute RHS and save it in a temporary variable so we can + // load it again later. ---

[PATCH] D146090: [Clang] Updating handling of defaulted comparison operators to reflect changes from P2448R2

shafik added a comment. In D146090#4319737 , @Mordante wrote: > In D146090#4319685 , @shafik wrote: > >> Landed fix 7887af027ee5eff27bbc953074726ab8d9d9f592 >>

cfe-commits@lists.llvm.org

shafik added a comment. In D148924#4318381 , @massberg wrote: > I have checked the paper P2002R1 and as far as I can tell it is fully > implemented when this patch has landed. So I read the paper but since the paper does not have examples for each chang

[PATCH] D149961: [Sema] Mark ineligibility of special member functions correctly

shafik added a comment. Thank you for fixing this so quickly! Comment at: clang/lib/Sema/SemaTemplateInstantiateDecl.cpp:2750 + if (auto *Constructor = dyn_cast(Method)) { +if (Constructor->isDefaultConstructor() || I have look at this function a few tim

[PATCH] D148689: [clang][Interp] Handle PredefinedExprs

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

[PATCH] D149694: [Clang] Update warning on some designator initializer cases involving unions

shafik updated this revision to Diff 519960. shafik added a comment. - Apply clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149694/new/ https://reviews.llvm.org/D149694 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaInit.cpp clang/test/Sema

[PATCH] D150036: [Clang] Correctly handle allocation in template arguments

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:15357 - if (!::EvaluateInPlace(Result.Val, Info, LVal, this) || Result.HasSideEffects) -return false; + { +FullExpressionRAII Scope(Info); Can you add a comment explaining why t

[PATCH] D150108: [clang] Evaluate non-type default template argument when it is required

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:1614 -TemplateArgument SugaredConverted, CanonicalConverted; -ExprResult DefaultRes = CheckTemplateArgument( Out of curiosity where is the template argument being checked now and

[PATCH] D149694: [Clang] Update warning on some designator initializer cases involving unions

This revision was automatically updated to reflect the committed changes. Closed by commit rG96bc78631f16: [Clang] Update warning on some designator initializer cases involving unions (authored by shafik). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D150075: Fix PR#62594 : static lambda call operator is not convertible to function pointer on win32

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

[PATCH] D146030: [clang][Interp] Handle LambdaExprs

shafik added inline comments. Comment at: clang/test/AST/Interp/lambda.cpp:5 +constexpr int a = 12; +constexpr int f = [c = a]() { return c; }(); +static_assert(f == a); Fun case ``` int constexpr f() { return [x = 10] { decltype(x) y; // type int b/c

[PATCH] D148851: Disable llvm-symbolizer on some of the driver tests that are timing out

shafik added a comment. In D148851#4328084 , @dblaikie wrote: > In D148851#4311266 , @ahatanak > wrote: > >> Disable llvm-symbolizer when running lit tests. > > This seems problematic though - when lit tests fail

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane, thakis, mstorsjo, rsmith. Herald added a project: All. shafik requested review of this revision. Previously we allowed the ability to downgrade this diagnostic based on feedback that time was needed to fix code: D131

[PATCH] D150209: [clang][Interp] Add more shift error checking

shafik added inline comments. Comment at: clang/lib/AST/Interp/Interp.h:133 +// operand, and must not overflow the corresponding unsigned type. +// C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to +// E1 x 2^E2 module 2^N. Maybe this qu

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

shafik updated this revision to Diff 520880. shafik added a comment. - Apply clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150226/new/ https://reviews.llvm.org/D150226 Files: clang/include/clang/Basic/DiagnosticASTKinds.td clang/lib/AST/ExprConstant.cpp clang/test/Se

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

shafik added a comment. I am wondering what section of the release notes to put this under. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150226/new/ https://reviews.llvm.org/D150226 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D150226: [Clang] Remove ability to downgrade warning on the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

shafik added a comment. Build failures look unrelated to my changes, it looks like it was failing before this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150226/new/ https://reviews.llvm.org/D150226 ___ cfe-commits mailing list cfe-commit

[PATCH] D150209: [clang][Interp] Add more shift error checking

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/AST/Interp/Interp.h:141-142 + + // C++2a [expr.shift]p2: E1 << E2 is the unique value congruent to + // E1 x 2^E2 module 2^N. +

<    5   6   7   8   9   10   11   12   >