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

shafik added a comment. ping @dim @rsmith @aaron.ballman CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148474/new/ https://reviews.llvm.org/D148474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

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

shafik added a comment. ping @dim @rsmith @aaron.ballman CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148474/new/ https://reviews.llvm.org/D148474 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[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] D159351: [Sema] Change order of displayed overloads in diagnostics

shafik added a comment. You say "attempts to be a strict weak order" does that mean there are still cases which will cause an assert or are we not sure? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159351/new/ https://reviews.llvm.org/D159351

[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] D148474: [Clang] Fix ResolveConstructorOverload to not select a conversion function if we are going use copy elision

shafik updated this revision to Diff 557569. shafik added a comment. - Integrate fix from https://github.com/llvm/llvm-project/pull/67373 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148474/new/ https://reviews.llvm.org/D148474 Files: clang/lib/Sema/SemaInit.cpp

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

shafik added a comment. @dim the crash should be fixed by landing: https://github.com/llvm/llvm-project/pull/67373 I need to rebase and test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148474/new/ https://reviews.llvm.org/D148474 ___

[PATCH] D84658: [clang] Overload NamedDecl::printName to provide a user-friendly name for unnamed entities

shafik added a comment. Herald added a project: All. It looks like some of these changes made it through different PRs but a lot of this is not in. It would be nice to get a updated version of this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D117197: [clang-format] Add new option to support adding a space between Javascript Union and Intersection types

shafik added a comment. Herald added a project: All. Herald added a reviewer: rymiel. Is this change still relevant or can we close this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117197/new/ https://reviews.llvm.org/D117197 ___

[PATCH] D116875: [clang-tidy] Add performance-inefficient-array-traversal check

shafik added a comment. Herald added a subscriber: PiotrZSL. Herald added a project: All. Is this change still relevant or can we close the PR? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D116875/new/ https://reviews.llvm.org/D116875

[PATCH] D117773: [clang-tidy] added JSON output to clang-tidy-diff.py

shafik added a comment. Herald added a reviewer: njames93. Herald added a subscriber: PiotrZSL. Herald added a project: All. Is this change still relevant of can we close this PR? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117773/new/

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

shafik added a comment. Note we have a second crash linked to this PR: https://github.com/llvm/llvm-project/issues/67173#issuecomment-1733647699 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139837/new/ https://reviews.llvm.org/D139837

[PATCH] D99466: Fix PR48889: assertion failure for void() in flattened ternary expression

shafik added a comment. Herald added a project: All. This is accepted but it looks like we need the previous `void{}` fix in order to land this. It would be nice to get both in before phab is not usable anymore. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D113838: Sema: Don't erroneously reject `void{}`

shafik added a comment. Herald added a project: All. It looks like this is almost there, can we get it over the line? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113838/new/ https://reviews.llvm.org/D113838

[PATCH] D75047: Add Control Flow Guard in Clang release notes.

shafik closed this revision. shafik added a comment. Herald added a project: All. Closing since this was landed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75047/new/ https://reviews.llvm.org/D75047

[PATCH] D69766: [Clang][MSVC] Use GetLinkerPath like the other toolchains for consistency

shafik added a comment. Herald added a subscriber: MaskRay. Herald added a project: All. @rnk can we close this PR? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69766/new/ https://reviews.llvm.org/D69766

[PATCH] D61663: [clang-format] Fix a JavaScript import order bug.

shafik added a comment. Herald added a project: All. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. @MyDeveloperDay this looks like it still could be relevant. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61663/new/

[PATCH] D7842: Make clang-format-diff compatible with Python 3.4

shafik closed this revision. shafik added a comment. Herald added a project: All. Closing this does not look relevant anymore CHANGES SINCE LAST ACTION https://reviews.llvm.org/D7842/new/ https://reviews.llvm.org/D7842 ___ cfe-commits mailing list

[PATCH] D55044: [clang-tidy] check for Abseil make_unique

shafik closed this revision. shafik added a comment. Herald added a subscriber: StephenFan. Herald added a project: All. Closing as it look like this is now `modernize-make-unique` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55044/new/ https://reviews.llvm.org/D55044

[PATCH] D61309: [clang] Add no-warn support for Wa

shafik closed this revision. shafik added a comment. Herald added subscribers: ormris, MaskRay. Herald added a project: All. Closing since these changes looks like they have landed already. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61309/new/

[PATCH] D18360: Add AIX Target/ToolChain to Clang Driver

shafik added a comment. Herald added a subscriber: steven.zhang. Herald added a project: All. @hubert.reinterpretcast is this still relevant? It looks like portions of this have landed, can we close it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D18360/new/

[PATCH] D45234: CMake: Check LLVM_ENABLE_LIBXML2 in clang

shafik added a comment. Herald added a project: All. Is this change still relevant of can we close this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45234/new/ https://reviews.llvm.org/D45234 ___ cfe-commits mailing

[PATCH] D156711: [clang][ExprConstant] Fix assertion failure in constant expression folding

shafik added a comment. It looks like I fixed this in: https://reviews.llvm.org/D158557 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156711/new/ https://reviews.llvm.org/D156711 ___ cfe-commits mailing

[PATCH] D23130: [Clang-tidy] Add a check for definitions in the global namespace.

shafik added a comment. Herald added a subscriber: wangpc. Herald added a project: All. Is this still relevant? It looks like some of these changes made it in. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D23130/new/ https://reviews.llvm.org/D23130

[PATCH] D41145: git-clang-format: refactor to support upcoming --staged flag

shafik added a comment. Herald added a subscriber: wangpc. Herald added a project: All. Is this still relevant or can we close this? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41145/new/ https://reviews.llvm.org/D41145 ___ cfe-commits

[PATCH] D41130: git-clang-format: cleanup: Use run() when possible.

shafik added a comment. Herald added a project: All. Is this still relevant or can we close it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D41130/new/ https://reviews.llvm.org/D41130 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D19201: [clang-tidy] misc-throw-with-noexcept

shafik added a comment. Herald added a project: All. Is this PR still relevant or can we close it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D19201/new/ https://reviews.llvm.org/D19201 ___ cfe-commits mailing list

[PATCH] D37196: [Clang] Bug 32352 - Provide a way for OptimizationRemarkEmitter::allowExtraAnalysis to check if (specific) remarks are enabled

shafik closed this revision. shafik added a comment. Herald added a project: All. It looks like these changes are already landed so closing CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37196/new/ https://reviews.llvm.org/D37196 ___

[PATCH] D37192: [clang-format] Add support for generic Obj-C categories

shafik added subscribers: Eugene.Zelenko, rjmccall, shafik. shafik added a comment. Herald added projects: All, clang, clang-format. Herald added reviewers: rymiel, HazardyKnusperkeks, owenpan, MyDeveloperDay. @rjmccall @Eugene.Zelenko this looks like it might still be relevant. CHANGES SINCE

[PATCH] D37004: [clang-diff] Fix the html output for CXXOperatorCallExpr

shafik added a comment. Herald added a project: All. Is this relevant anymore or should we close it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D37004/new/ https://reviews.llvm.org/D37004 ___ cfe-commits mailing list

[PATCH] D22189: llvm.noalias - Clang CodeGen - check restrict variable map only for restrict-qualified lvalues

shafik added a comment. Herald added subscribers: mattd, gchakrabarti, asavonic, jeroen.dobbelaere, bollu. Herald added a project: All. @rjmccall is this still relevant or can we close it? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D22189/new/ https://reviews.llvm.org/D22189

[PATCH] D18478: python bindings: expose the clang version string

shafik added a comment. Herald added a subscriber: arphaman. Herald added a project: All. @compnerd is this still relevant? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D18478/new/ https://reviews.llvm.org/D18478 ___ cfe-commits mailing list

[PATCH] D28144: clang support for Mageia 6 distro

shafik added a comment. Herald added subscribers: pengfei, kristof.beyls. Herald added a project: All. @bkramer is this still relevant or should it be closed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D28144/new/ https://reviews.llvm.org/D28144

[PATCH] D22334: Fix for Bug 28172 : clang crashes on invalid code (with too few arguments to __builtin_signbit) without any proper diagnostics.

shafik added a comment. Herald added a project: All. It looks like the examples no longer crash: https://godbolt.org/z/Yn8qK3fnT So likely we should just close this. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D22334/new/ https://reviews.llvm.org/D22334

[PATCH] D25171: clang-format: Add two new formatting options

shafik added a comment. Herald added a project: All. Is this still relevant or should this be closed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D25171/new/ https://reviews.llvm.org/D25171 ___ cfe-commits mailing list

[PATCH] D19385: [scan-build] fix warnings emitted on Clang Format code base

shafik added a comment. Herald added a project: All. Is this relevant anymore or should it be closed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D19385/new/ https://reviews.llvm.org/D19385 ___ cfe-commits mailing list

[PATCH] D23602: Port tools/clang-format/git-clang-format to work Python beyond 2.7

shafik added a comment. Herald added a project: All. Should we close this? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D23602/new/ https://reviews.llvm.org/D23602 ___ cfe-commits mailing list

[PATCH] D11235: clang-format: Fix breaking before nested 'operator' in function declarations

shafik added a comment. Herald added a project: All. @Eugene.Zelenko it this still relevant or should this be closed? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D11235/new/ https://reviews.llvm.org/D11235 ___ cfe-commits mailing list

[PATCH] D24439: [Clang] Fix some Clang-tidy modernize-use-bool-literals and Include What You Use warnings; other minor fixes

shafik added a comment. Herald added subscribers: ChuanqiXu, arphaman. Herald added a project: All. Is this PR still relevant or should it just be closed? Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D24439/new/ https://reviews.llvm.org/D24439

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

shafik added a comment. In D150075#4646487 , @royjacobson wrote: > @shafik @aaron.ballman does any of you want to commandeer this diff? seems > simple enough of a change I think Aaron might be better since he is more familiar with Windows.

[PATCH] D158804: [Clang] Fix crash in Parser::ParseDirectDeclarator by adding check that token is not an annotation token

This revision was automatically updated to reflect the committed changes. Closed by commit rG6eadc8f16e03: [Clang] Fix crash in Parser::ParseDirectDeclarator by adding check that token… (authored by shafik). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D158804: [Clang] Fix crash in Parser::ParseDirectDeclarator by adding check that token is not an annotation token

shafik updated this revision to Diff 556590. shafik added a comment. - Add release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158804/new/ https://reviews.llvm.org/D158804 Files: clang/docs/ReleaseNotes.rst clang/lib/Parse/ParseDecl.cpp clang/test/Parser/gh64836.cpp

[PATCH] D159345: [Clang] Handle non-ASCII after line splicing

shafik added a comment. I think this looks good but I would like @tahonermann to review this as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159345/new/ https://reviews.llvm.org/D159345 ___

[PATCH] D159345: [Clang] Handle non-ASCII after line splicing

shafik added inline comments. Comment at: clang/lib/Lex/Lexer.cpp:1757 + // If a UTF-8 codepoint appears immediately after an escaped new line, + // CurPtr may point to the splicing \ at the on the preceding line, + // so we need to skip it. I think that is

[PATCH] D159247: [HLSL] Cleanup support for `this` as an l-value

shafik added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:2174 + static QualType getThisType(const FunctionProtoType *FPT, cor3ntin wrote: > Whitespace only change Spurious change. Comment at:

[PATCH] D159133: [Sema] Make C++ functional-style cast warn about dropped qualifiers (-Wcast-qual)

shafik added a comment. I think this makes sense especially if it matches up w/ gcc but would like to see more feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159133/new/ https://reviews.llvm.org/D159133

[PATCH] D159133: [Sema] Make C++ functional-style cast warn about dropped qualifiers (-Wcast-qual)

shafik added reviewers: aaron.ballman, clang-language-wg. shafik added a comment. Add more reviewers for visibility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D159133/new/ https://reviews.llvm.org/D159133

[PATCH] D158808: [Clang] Modify Parser::ParseLambdaExpressionAfterIntroducer to check whether the lambda-declarator is valid

This revision was automatically updated to reflect the committed changes. Closed by commit rG6f30ef360127: [Clang] Modify Parser::ParseLambdaExpressionAfterIntroducer to check whether… (authored by shafik). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D159083: Clang: Don't warn about unused private fields of types declared maybe_unused

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

[PATCH] D158808: [Clang] Modify Parser::ParseLambdaExpressionAfterIntroducer to check whether the lambda-declarator is valid

shafik updated this revision to Diff 554345. shafik added a comment. - Add release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158808/new/ https://reviews.llvm.org/D158808 Files: clang/docs/ReleaseNotes.rst clang/lib/Parse/ParseExprCXX.cpp

[PATCH] D158933: [clang] Implement -funsigned-bitfields

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

[PATCH] D158615: [clang] Emit an error if variable ends up with incomplete array type

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. I Think this change makes sense. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158615/new/ https://reviews.llvm.org/D158615

[PATCH] D158601: [Clang] Always constant-evaluate operands of comparisons to nullptr

shafik added a comment. It might have been nice to add a test where `nullptr_t` comparison succeeds as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158601/new/ https://reviews.llvm.org/D158601

[PATCH] D158808: [Clang] Modify Parser::ParseLambdaExpressionAfterIntroducer to check whether the lambda-declarator is valid

shafik updated this revision to Diff 553692. shafik added a comment. - Move test to C++20 file - Apply clang-format CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158808/new/ https://reviews.llvm.org/D158808 Files: clang/lib/Parse/ParseExprCXX.cpp

[PATCH] D158557: [clang] Fix crash in __builtin_strncmp and other related builtin functions

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG33b6b674620d: [clang] Fix crash in __builtin_strncmp and other related builtin functions (authored by shafik). Herald added a project: clang.

[PATCH] D158557: [clang] Fix crash in __builtin_strncmp and other related builtin functions

shafik updated this revision to Diff 553601. shafik added a comment. - Extended constexpr-string.cpp test - Added release notes CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158557/new/ https://reviews.llvm.org/D158557 Files: clang/docs/ReleaseNotes.rst

[PATCH] D158827: [clang] Fix assertion fail when function has cleanups and fatal errors

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

[PATCH] D158808: [Clang] Modify Parser::ParseLambdaExpressionAfterIntroducer to check whether the lambda-declarator is valid

shafik created this revision. shafik added reviewers: cor3ntin, aaron.ballman, erichkeane, rsmith. Herald added a project: All. shafik requested review of this revision. We had a couple of crashes due to invalid lambda trailing return types that were diagnosed but not treated as errors during

[PATCH] D158804: [Clang] Fix crash in Parser::ParseDirectDeclarator by adding check that token is not an annotation token

shafik created this revision. shafik added reviewers: aaron.ballman, nridge, erichkeane. Herald added a project: All. shafik requested review of this revision. In `Parser::ParseDirectDeclarator(...)` in some cases ill-formed code can cause an annotation token to end up where it was not expected.

[PATCH] D158135: [Clang][CodeGen] Add __builtin_bcopy

shafik added inline comments. Comment at: clang/lib/CodeGen/CGBuiltin.cpp:3567 + case Builtin::BI__builtin_bcopy: { +Address Dest = EmitPointerWithAlignment(E->getArg(1)); +Address Src = EmitPointerWithAlignment(E->getArg(0)); Maybe it is better to do

[PATCH] D158557: [clang] Fix crash in __builtin_strncmp and other related builtin functions

shafik updated this revision to Diff 553270. shafik marked 2 inline comments as done. shafik added a comment. - Add codegen test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158557/new/ https://reviews.llvm.org/D158557 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D158557: [clang] Fix crash in __builtin_strncmp and other related builtin functions

shafik added inline comments. Comment at: clang/test/SemaCXX/constexpr-string.cpp:681 +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wconstant-conversion" +namespace GH64876 { erichkeane wrote: > rather than suppress these, it makes sense to

[PATCH] D158557: [clang] Fix crash in __builtin_strncmp and other related builtin functions

shafik updated this revision to Diff 553195. shafik added a comment. - Silence -Wconstant-conversion warning in test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158557/new/ https://reviews.llvm.org/D158557 Files: clang/lib/AST/ExprConstant.cpp

[PATCH] D158733: [Clang] Fix a crash when an invalid immediate function call appears in a cast

shafik added inline comments. Comment at: clang/test/SemaCXX/cxx2a-consteval.cpp:1120 + +void k() { (int)h{nullptr}; } +// expected-error@-1 {{call to consteval function 'GH64949::h::h' is not a constant expression}} Another variety maybe worthing adding a test

[PATCH] D157855: [clang][ExprConstant] Improve error message of compound assignment against uninitialized object

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

[PATCH] D158526: [clang] Properly print unnamed members in diagnostics

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

[PATCH] D139837: [Clang] Implements CTAD for aggregates P1816R0 and P2082R1

shafik added a comment. So while working on D148474 I realized this PR introduced a new crash bug, see the following code: https://godbolt.org/z/h1EezGjbr template class B3 : A3 { template()> B3(); }; B3(); which is one of my test

[PATCH] D157296: [AST][Coroutine] Fix CoyieldExpr missing end loc

shafik added inline comments. Comment at: clang/lib/Sema/SemaCoroutine.cpp:322 + auto EndLoc = Args.empty() ? Loc : Args.back()->getEndLoc(); + return S.BuildCallExpr(nullptr, Result.get(), Loc, Args, EndLoc, nullptr); } We should use

[PATCH] D158557: [clang] Fix crash in __builtin_strncmp and other related builtin functions

shafik added a comment. In D158557#4608262 , @erichkeane wrote: > Are there any Sema tests we can add to show that we warn/diagnose/SOMETHING > on these? If someone passes a negative size, we should probably at least do > the warning that it was

[PATCH] D158557: [clang] Fix crash in __builtin_strncmp and other related builtin functions

shafik updated this revision to Diff 552518. shafik added a comment. -Updated values used in test CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158557/new/ https://reviews.llvm.org/D158557 Files: clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX/constexpr-string.cpp Index:

[PATCH] D158557: [clang] Fix crash in __builtin_strncmp and other related builtin functions

shafik updated this revision to Diff 552516. shafik added a comment. - Diff w/ context CHANGES SINCE LAST ACTION https://reviews.llvm.org/D158557/new/ https://reviews.llvm.org/D158557 Files: clang/lib/AST/ExprConstant.cpp clang/test/SemaCXX/constexpr-string.cpp Index:

[PATCH] D158557: [clang] Fix crash in __builtin_strncmp and other related builtin functions

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane, davide, rsmith. Herald added a project: All. shafik requested review of this revision. The implementation of `__builtin_strncmp` and other related builtins function use `getExtValue()` to evaluate the size argument.

[PATCH] D157252: [clang][ExprConst] Handle 0 type size in builtin_memcpy etc.

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:9512 uint64_t TSize = Info.Ctx.getTypeSizeInChars(T).getQuantity(); +if (TSize == 0) + return false; I think we should issue a diagnostic, we don't have any indication that

[PATCH] D158526: [clang] Properly print unnamed members in diagnostics

shafik added a comment. Thank you for this fix, it is a lot cleaner than I was expecting. Comment at: clang/include/clang/AST/Decl.h:3186 + + void printName(raw_ostream , const PrintingPolicy ) const override; }; So it looks like w/o this we would end up

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

shafik added a comment. In D148474#4490041 , @dim wrote: > FWIW, this fix works for the test cases I had via > https://bugs.freebsd.org/269067 and > https://github.com/llvm/llvm-project/issues/60182, but I got the following > failure during check-all:

[PATCH] D156565: Diagnose use of VLAs in C++ by default

shafik added a comment. In D156565#4599812 , @aaron.ballman wrote: > Enable the diagnostic by default in C++ language modes, and under -Wall in > GNU++ language modes. I am happy with that approach. CHANGES SINCE LAST ACTION

[PATCH] D156693: [clang][ASTImporter]Skip check depth of friend template parameter

shafik added inline comments. Comment at: clang/unittests/AST/StructuralEquivalenceTest.cpp:140 +D0->getASTContext(), D1->getASTContext(), NonEquivalentDecls01, +StructuralEquivalenceKind::Default, false, false, false, +IgnoreTemplateParmDepth);

[PATCH] D158433: [Clang] Do not change the type of captured vars when checking lambda constraints

shafik added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:722 +const_cast(cast(FD))); +// Captures are not checked from within the lambda. +LSI->AfterParameterList = false; This comment does not really explain to me the effect of

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGcc1b6668c571: [Clang] Fix member lookup so that we dont ignore ambiguous lookups in some… (authored by shafik). Herald added a project: clang.

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

shafik updated this revision to Diff 545283. shafik added a comment. Add release note. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155387/new/ https://reviews.llvm.org/D155387 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Sema/Lookup.h clang/lib/Sema/SemaOverload.cpp

[PATCH] D156482: [clang][CGExprConstant] handle FunctionToPointerDecay

shafik added a comment. Makes sense but I want @efriedma to look at as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156482/new/ https://reviews.llvm.org/D156482 ___ cfe-commits mailing list

[PATCH] D156201: [ASTImporter] Fix corrupted RecordLayout introduced by circular referenced fields

shafik added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:7403 + UO->setOperatorLoc(ToOperatorLoc); + UO->setCanOverflow(E->canOverflow()); + I don't see the following values from the old code used: `E->getValueKind()`, `E->getObjectKind()` and

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

shafik updated this revision to Diff 544999. shafik added a comment. rebase CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155387/new/ https://reviews.llvm.org/D155387 Files: clang/include/clang/Sema/Lookup.h clang/lib/Sema/SemaOverload.cpp clang/lib/Sema/SemaTemplate.cpp

[PATCH] D156093: [ASTImporter] Re-odering by lexical order for all imported decls within record

shafik added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:1950 +ToD && ToDC == ToD->getLexicalDeclContext() && +ToDC->containsDecl(ToD)) { + ToDC->removeDecl(ToD); If `ToDC` does not contain the decl is that a problem, what

[PATCH] D154764: [ASTImporter] Fields are imported first and reordered for correct layout.

shafik added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:8018 +int m() { + return &((A *)0)->f1 - &((A *)0)->f2; +} So is it the case that this caused `f2` to be imported first and then `f1`? Which is the

[PATCH] D156466: [clang][CGExprConstant] handle implicit widening/narrowing Int-to-Int casts

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

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

shafik marked 4 inline comments as done. shafik added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:15191 OverloadCandidateSet::iterator Best; switch (CandidateSet.BestViableFunction(*this, OpLoc, Best)) { case OR_Success: rsmith wrote: >

[PATCH] D155387: [Clang] Fix member lookup so that we don't ignore ambiguous lookups in some cases

shafik updated this revision to Diff 544960. shafik marked an inline comment as done. shafik added a comment. - Updated so that we ignore ambiguous overload candidate if the name lookup was also ambiguous. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155387/new/

[PATCH] D155661: [ASTImporter] Fix friend class template import within dependent context

shafik added a comment. This mostly makes sense to me, but I want another review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155661/new/ https://reviews.llvm.org/D155661 ___ cfe-commits mailing list

[PATCH] D156337: [clang] Allow setting the uninitialized attribute on record

shafik added a comment. I think a test verifying that during constant evaluation we still flag the read of a local tagged uninitialized as ill-formed would be nice. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156337/new/

[PATCH] D156337: [clang] Allow setting the uninitialized attribute on record

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

[PATCH] D156247: [Clang] Add a warning on uses of coroutine keywords

shafik added a comment. In D156247#4536034 , @tahonermann wrote: >> You are absolutely right, -fno-coroutines would totally work for us, had it >> been available. > > Good. Gcc handles `-fcoroutines` and `-fno-coroutines` as I would expect >

[PATCH] D156224: [Clang] use unsigned integer constants in unit-test | fixes build error on ppc64le-lld-multistage-test

shafik added inline comments. Comment at: clang/unittests/libclang/LibclangTest.cpp:1232 ASSERT_TRUE(staticAssertCsr.has_value()); - size_t argCnt = 0; + int argCnt = 0; Traverse(*staticAssertCsr, [](CXCursor cursor, CXCursor parent) { Above you used

[PATCH] D153689: [clang][Interp] Handle CXXConstructExprs

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1109 +template +bool ByteCodeExprGen::VisitCXXConstructExpr( +const CXXConstructExpr *E) { Should we be checking `isElidable()`? Repository: rG LLVM Github Monorepo

[PATCH] D153653: [clang][Interp] Make CXXTemporaryObjectExprs leave a value behind

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

[PATCH] D156212: [clang][Interp] Implement remaining strcmp builtins

shafik added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:17 + static_assert(__builtin_strncmp("abaa", "abba", 1) == 0); + static_assert(__builtin_strncmp("abaa", "abba", 0) == 0); + static_assert(__builtin_strncmp(0, 0, 0) == 0);

[PATCH] D156307: [clang][DeclPrinter] Fix AST print of curly constructor initializers

shafik added a comment. Maybe add a designated init test in there as well e.g. struct A { int x {}; }; struct B { B() : a({.x = 1}) { } A a; }; Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156307/new/

[PATCH] D156244: [clang] Do not crash on use of a variadic overloaded operator

shafik added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:14001 + +if (FnDecl->isInvalidDecl()) + return ExprError(); aaron.ballman wrote: > Fznamznon wrote: > > Fznamznon wrote: > > > shafik wrote: > > > > Fznamznon wrote: > > >

[PATCH] D156244: [clang] Do not crash on use of a variadic overloaded operator

shafik added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:14001 + +if (FnDecl->isInvalidDecl()) + return ExprError(); Fznamznon wrote: > shafik wrote: > > shafik wrote: > > > It feels a bit weird that we are succeeding in finding

[PATCH] D156244: [clang] Do not crash on use of a variadic overloaded operator

shafik added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:14001 + +if (FnDecl->isInvalidDecl()) + return ExprError(); shafik wrote: > It feels a bit weird that we are succeeding in finding the best viable > function and what we

  1   2   3   4   5   6   7   8   9   10   >