[PATCH] D155705: [clang] Fix specialization of non-templated member classes of class templates

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

[PATCH] D154893: [Clang] Fix some triviality computations

shafik added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:1269 /// Determine whether this class has a non-trivial copy constructor /// (C++ [class.copy]p6, C++11 [class.copy]p12) bool hasNonTrivialCopyConstructor() const { philnik wrote

[PATCH] D156064: [SemaCXX] Recognise initializer_list injected-class-name types as initializer_lists

shafik added a comment. This makes sense I see that `isStdInitializerList(...)` is used in a lot of places, this makes me wonder if we need additional test coverage. I also added some folks for more review visibility. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D156154: [clang][ConstExprEmitter] handle IntegerLiterals

shafik added a comment. What practical effects will this have? What kind of code do this effect? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D156154/new/ https://reviews.llvm.org/D156154 ___ cfe-commits

[PATCH] D156154: [clang][ConstExprEmitter] handle IntegerLiterals

shafik added a comment. In D156154#4529907 , @nickdesaulniers wrote: > > Should speed up "is the below expression a constant expression?" > >> What kind of code do this effect? > > > > int y = 2; > ^ is 2 a constant expression? Nice! R

[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(); It feels a bit weird that we are succeeding in finding the best viable function and what we find is invalid.

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

[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(); aaron.ballman wrote: > Fznamznon wrote: > > Fznamznon wrote: > > > shafik wrote: > > > > Fznamznon wrote: > > >

[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/ https://revie

[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); H

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

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

[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, [&argCnt](CXCursor cursor, CXCursor parent) { Above you us

[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 > (https://g

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

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

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

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

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

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

[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 clang/

[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] 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 cfe-co

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

[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] 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 &OS, const PrintingPolicy &Policy) const override; }; So it looks like w/o this we would e

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

[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] 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: clang/test/SemaCX

[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: clang/

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

[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 [bugprone-argument-comme

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

[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] 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] 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] 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 don't ignore ambiguous lookups in some… (authored by shafik). Herald added a project: clang. Repo

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

[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] 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. Repository

[PATCH] D153375: [Clang] Fix incorrect use of direct initialization with copy initialization

shafik updated this revision to Diff 536451. shafik marked an inline comment as done. shafik added a comment. - Added release note - Added test case CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153375/new/ https://reviews.llvm.org/D153375 Files: clang/docs/ReleaseNotes.rst clang/l

[PATCH] D154334: [clang] Add `__has_extension ()` for C++11 features

shafik added a comment. Thank you or the quick fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154334/new/ https://reviews.llvm.org/D154334 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D153296: [AST] Stop evaluate constant expression if the condition expression which in switch statement contains errors

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:5019 if (SS->getCond()->isValueDependent()) { if (!EvaluateDependentExpr(SS->getCond(), Info)) return ESR_Failed; Please don't forget to remove this `if` and make the re

[PATCH] D153375: [Clang] Fix incorrect use of direct initialization with copy initialization

shafik added inline comments. Comment at: clang/test/CXX/dcl.decl/dcl.init/dcl.init.list/p3.cpp:293 + e = {E::E1}; + e = {0}; // expected-error {{cannot initialize a value of type 'E' with an rvalue of type 'int'}} +} rsmith wrote: > This looks valid to me. Th

[PATCH] D84364: [CUDA][HIP] Defer overloading resolution diagnostics for host device functions

shafik added inline comments. Herald added subscribers: jplehr, mattd, carlosgalvezp, MaskRay. Herald added a project: All. Comment at: clang/lib/Sema/SemaDecl.cpp:14543 // deletion in some later function. -if (getDiagnostics().hasUncompilableErrorOccurred() || +if (

[PATCH] D152495: [Clang][SemaCXX] Add unused warning for variables declared in condition expressions

shafik added inline comments. Comment at: clang/lib/Sema/SemaExprCXX.cpp:4003-4004 + // that are not referenced or used later. e.g.: if (int var = init()); + ConditionVar->setReferenced(false); + ConditionVar->setIsUsed(false); + tbaeder wrote: > shafik wrote:

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik created this revision. shafik added reviewers: aaron.ballman, cor3ntin. Herald added a project: All. shafik requested review of this revision. In C++ we are not allowed to use designated initializers to initialize fields out of order. In some cases when diagnosing this we are crashing beca

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik added inline comments. Comment at: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp:195 +.c = 1, // reorder-error {{field 'd' will be initialized after field 'c'}} // reorder-note {{previous initialization for field 'c' is here}} +.b = 1, // reorder-error {{fie

[PATCH] D154689: [clang] Correct calculation of MemberExpr's dependence

shafik accepted this revision. shafik added a comment. LGTM, thank you for the quick fix! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154689/new/ https://reviews.llvm.org/D154689 ___ cfe-commits mailin

[PATCH] D154696: [Clang] Diagnose jumps into statement expressions

shafik added inline comments. Comment at: clang/docs/ReleaseNotes.rst:571 (`#61758 `_) +- Correcly diagnose jumps into statement expressions. + (`#63682 `_)

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

shafik added reviewers: aaron.ballman, clang-language-wg. shafik added a comment. For more visibility. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154503/new/ https://reviews.llvm.org/D154503 ___ cfe-commits mailing list cfe-commits@lists.l

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik updated this revision to Diff 538323. shafik marked an inline comment as done. shafik added a comment. -Add fix for wrong field in diagnostic CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154675/new/ https://reviews.llvm.org/D154675 Files: clang/lib/Sema/SemaInit.cpp clang/t

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

shafik added inline comments. Comment at: clang/test/SemaCXX/using-hiding.cpp:20 + +// Using declaration causes A::X to be hidden, so X is not ambiguous. +namespace Test2 { I think [namespace.udecl p10](https://eel.is/c++draft/namespace.udecl#10) disagrees, spec

[PATCH] D154503: [Sema] Fix handling of functions that hide classes

shafik added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:507 + // C++ [basic.scope.hiding]p2: + // A class name or enumeration name can be hidden by the name of This section does not exist anymore, it was replaced in [p1787](https://wg21.lin

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik added a comment. I can't really see any details on the libcxx failure :-( CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154675/new/ https://reviews.llvm.org/D154675 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik added inline comments. Comment at: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp:182 +namespace GH63605 { +struct { + unsigned : 2; cor3ntin wrote: > Should we add bases? Good catch b/c of course bases would cause problems 🤣 CHANGES SINCE LAST ACT

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik updated this revision to Diff 538456. shafik added a comment. - Stopped using `NumBases` when calculating `OldIndex` - Added bases classes to the test and added more test cases CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154675/new/ https://reviews.llvm.org/D154675 Files: cl

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik marked an inline comment as done. shafik added a comment. Found a new issue https://github.com/llvm/llvm-project/issues/63759 but this feels different enough that I will deal with it separately. Comment at: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp:182 +namesp

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik added a comment. @aaron.ballman I can't figure out why the libcxx bot failed, can you understand why? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154675/new/ https://reviews.llvm.org/D154675 ___ cfe-commits mailing list cfe-commits@

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik added inline comments. Comment at: clang/test/SemaCXX/cxx2a-initializer-aggregates.cpp:66 .y = 1, // override-note {{previous}} - .y = 1, // override-error {{overrides prior initialization}} + .y = 1, // override-error {{overrides prior initialization}} // reorder-not

[PATCH] D154861: [clang][AST] Propagate the contains-errors bit to DeclRefExpr from VarDecl's initializer.

shafik added inline comments. Comment at: clang/lib/AST/ComputeDependence.cpp:461 /// based on the declaration being referenced. ExprDependence clang::computeDependence(DeclRefExpr *E, const ASTContext &Ctx) { auto Deps = ExprDependence::None; `computeDepen

[PATCH] D41416: [modules] [pch] Do not deserialize all lazy template specializations when looking for one.

shafik added inline comments. Comment at: clang/lib/AST/DeclTemplate.cpp:337 +void RedeclarableTemplateDecl::loadLazySpecializationsImpl( + bool OnlyPartial/*=false*/) const { // Grab the most recent declaration to ensure we've load

[PATCH] D154893: [Clang] Fix some triviality computations

shafik added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:1269 /// Determine whether this class has a non-trivial copy constructor /// (C++ [class.copy]p6, C++11 [class.copy]p12) bool hasNonTrivialCopyConstructor() const { These referen

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

shafik added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:167 +static_assert(false, MessageInvalidSize{}); // expected-error {{static assertion failed}} \ + // expected-error {{the message in a static_asser

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

shafik added inline comments. Comment at: clang/test/SemaCXX/static-assert-cxx26.cpp:1 +// RUN: %clang_cc1 -std=c++2c -fsyntax-only %s -verify + How about a test case w/ `data()` and or `size()` has default arguments. Repository: rG LLVM Github Monorepo CHAN

[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded

shafik added inline comments. Comment at: clang/test/SemaCXX/lambda-pack-expansion.cpp:36 + +template void f(); + Can we also have an example similar to the bug report where we have multiple arguments. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D154716: [SemaCXX] Fix bug where unexpanded lambda captures where assumed to be expanded

shafik added inline comments. Comment at: clang/lib/Sema/TreeTransform.h:13323 // If this is an init-capture pack, consider expanding the pack now. if (OldVD->isParameterPack()) { Based on the changes is this comment still accurate? Repository: rG

[PATCH] D146178: [Clang][Sema] Fix comparison of constraint expressions

shafik added a comment. Looks like this bug is caused by this commit: https://github.com/llvm/llvm-project/issues/63782#issuecomment-1633312909 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146178/new/ https://reviews.llvm.org/D146178 ___

[PATCH] D154290: [Clang] Implement P2741R3 - user-generated static_assert messages

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:16408 +Char) || +!Char.isInt()) + return false; Why are we specifically checking `!isInt()` what `Kind` do we expect?

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

shafik updated this revision to Diff 540587. shafik marked 2 inline comments as done. shafik added a comment. - Add release notes. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154675/new/ https://reviews.llvm.org/D154675 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaInit.c

[PATCH] D154675: [Clang] Fix crash when emitting diagnostic for out of order designated initializers in C++

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc9ef33e1d8a8: [Clang] Fix crash when emitting diagnostic for out of order designated… (authored by shafik). Herald added a project: clang. Repositor

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

shafik created this revision. shafik added reviewers: aaron.ballman, rsmith, cor3ntin. Herald added a project: All. shafik requested review of this revision. There are some cases during member lookup we are aggressively suppressing diagnostics when we should just be suppressing access control dia

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

shafik added a comment. It looks like the paragraphs in `class.member.lookup` have changed a lot and so the tests don't totally match in that directory. Fixing that seems like a big piece of work but maybe worth doing. Comment at: clang/include/clang/Sema/Lookup.h:152 +

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:14930 LookupQualifiedName(R, Record->getDecl()); - R.suppressDiagnostics(); + R.suppressAccessDiagnostics(); rsmith wrote: > shafik wrote: > > I was a bit conservative where I change

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaOverload.cpp:14930 LookupQualifiedName(R, Record->getDecl()); - R.suppressDiagnostics(); + R.suppressAccessDiagnostics(); shafik wrote: > rsmith wrote: > > shafik wrote: > > > I was a bit conserv

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

shafik updated this revision to Diff 540866. shafik added a comment. - Adjusted more place to use `suppressAccessDiagnostics` 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/SemaOverl

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

[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] 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 clang/test/SemaCXX/constexpr-string.

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

[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 clang/test/CodeGen/gh648

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

[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] 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 par

[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] 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 clang/lib/AST/ExprConstant.c

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

[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 clang/test/Parser/cxx2a-template-la

[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] 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] 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 cf

[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 clang/test/Parser/cxx2a-template

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

[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

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