[PATCH] D144866: [clang] Fix aggregate initialization inside lambda constexpr

shafik added a comment. Also please add a release note before landing the fix, thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144866/new/ https://reviews.llvm.org/D144866 ___ cfe-commits mailin

[PATCH] D144878: __builtin_FILE_NAME()

shafik added inline comments. Comment at: clang/lib/AST/Expr.cpp:2283 + case SourceLocExpr::FileName: { +SmallString<256> Path; +// builtin_FILE_NAME() is a Clang-specific extension that expands to the It looks like a copy of the code from `ExpandBuiltin

[PATCH] D144272: [clang][Interp] Ignore StaticAssertDecls

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

[PATCH] D144866: [clang] Fix aggregate initialization inside lambda constexpr

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM, the modules failures is a previous known issue. Comment at: clang/lib/AST/ExprConstant.cpp:8763 if (isLambdaCallOperator(Info.CurrentCall->Callee)) { - // Ens

[PATCH] D144866: [clang] Fix aggregate initialization inside lambda constexpr

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8763 if (isLambdaCallOperator(Info.CurrentCall->Callee)) { - // Ensure we actually have captured 'this'. (an error will have - // been previously reported if not). + // Ensure we actually

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

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

[PATCH] D144866: [clang] Fix aggregate initialization inside lambda constexpr

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8763 if (isLambdaCallOperator(Info.CurrentCall->Callee)) { - // Ensure we actually have captured 'this'. (an error will have - // been previously reported if not). + // Ensure we actually

[PATCH] D144866: [clang] Fix aggregate initialization inside lambda constexpr

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:8763 if (isLambdaCallOperator(Info.CurrentCall->Callee)) { - // Ensure we actually have captured 'this'. (an error will have - // been previously reported if not). + // Ensure we actually

[PATCH] D145034: [Clang][Sema] Preparations to fix handling of out-of-line definitions of constrained templates

shafik added inline comments. Comment at: clang/include/clang/Sema/DeclSpec.h:68 NestedNameSpecifierLocBuilder Builder; + ArrayRef TemplateParamLists; Does this really represent a `nested-name-specifier` with a `template-param-list`? Maybe I am misundersta

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

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

[PATCH] D140584: [Clang] Refactor "Designators" into a unified implementation [NFC]

shafik added a comment. I think these two bug reports: https://github.com/llvm/llvm-project/issues/46132 and https://github.com/llvm/llvm-project/issues/61118 may be related to this change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140584/new/

[PATCH] D127259: [CodeGen] guarantee templated static variables are initialized in the reverse instantiation order

shafik added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:19907 + if (Iter != I.end()) { +SemaRef.PendingInstantiations.push_back(*Iter); +I.erase(Iter); Doesn't this invalidate the iterators? Repository: rG LLVM

[PATCH] D145362: [clang] Update test according to P1937

shafik accepted this revision. shafik added a comment. Besides the nitpick, LGTM Comment at: clang/test/CXX/expr/expr.const/p8-2a.cpp:14 -// bad_assert_copyable is not needed for constant evaluation -// (and thus not instantiated) +// consteval assert is not instantiated as w

[PATCH] D145251: [clang] Treat function parameter scope as an immediate function context

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. This LGTM, I agree w/ @cor3ntin about quoting [expr.cont] to document this choice. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145251/new/ ht

[PATCH] D145793: [clang][AST] Improve diagnostic for `nullptr` constexpr function pointer call

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:7673 +Info.FFDiag(Callee, diag::note_constexpr_null_callee) +<< const_cast(Callee); +return false; hazohelet wrote: > tbaeder wrote: > > Is the `const_cast` reall

[PATCH] D145842: [clang][Sema] Avoid duplicate diagnostics for unreachable fallthrough attribute

shafik added inline comments. Comment at: clang/lib/Analysis/ReachableCode.cpp:674 + if (AS && hasSpecificAttr(AS->getAttrs()) && + !Diag.isIgnored(diag::warn_unreachable_fallthrough_attr, + SourceLocation())) { Your passing through `Di

[PATCH] D145851: [Clang][Sema] Fix incorrect deletion of default constructors for some unions

shafik added a comment. LGTM, I wlll let @royjacobson give the final approval after reviewing codegen tests. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:9161-9162 +if (CSM == Sema::CXXDefaultConstructor) { + // Default constructor of a union can also be valid if one of

[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.

shafik added inline comments. Comment at: clang/test/SemaCXX/type-traits.cpp:2886-2889 +struct UnnamedEmptyBitfield { + int named; + int : 0; +}; aaron.ballman wrote: > I think there's one more test to add: > ``` > struct UnnamedEmptyBitfieldSplit { > short n

[PATCH] D153294: [clang] Do not create ExprWithCleanups while checking immediate invocation

shafik added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:18177 getASTContext()), /*IsImmediateInvocation*/ true); if (Cached.hasValue()) Drive by nit clean-up while we are here. Comment

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

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane, rsmith. Herald added a project: All. shafik requested review of this revision. In `Sema::CreateBuiltinBinOp()` we were incorrectly using direct initialization instead of copy initialization. This led to a crash in a

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

shafik added a comment. It looks like the build failure happens on other PR as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153375/new/ https://reviews.llvm.org/D153375 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

shafik added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11226 +def err_module_private_use : Error< + "%select{declaration|definition|default argument|" + "explicit specialization|partial specialization}0 of %1 is private to module " -

[PATCH] D145965: [C++20][Modules] Fix incorrect visibilities in implementation units.

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

[PATCH] D152788: [Clang] Show type in enum out of range diagnostic

shafik added a comment. Thank you for this improvement! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152788/new/ https://reviews.llvm.org/D152788 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D152796: [clang][Sema] Fix diagnostic message for unused constant varialbe templates

shafik added inline comments. Comment at: clang/lib/Sema/Sema.cpp:1385 + Diag(DiagD->getLocation(), diag::warn_unused_template) + << /*variable*/ 1 << DiagD; } else if (DiagD->getType().isConstQualified()) { Sorry for late review bu

[PATCH] D148697: [clang-tidy] Add more checks for functions which should be noexcept

shafik added inline comments. Comment at: clang-tools-extra/clang-tidy/utils/LexerUtils.cpp:273 +return Lexer::findLocationAfterToken(NoexceptLoc, tok::r_paren, SM, + LangOpts, true); + To conform with [burprone-argume

[PATCH] D151720: [clang][ExprConstant] Fix display of syntactically-invalid note for member function calls

shafik added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:983 EnableNewConstInterp(C.getLangOpts().EnableNewConstInterp), - BottomFrame(*this, SourceLocation(), nullptr, nullptr, CallRef()), + BottomFrame(*this, SourceLocation(), nullpt

[PATCH] D152946: [C++20][Modules] Implement P2615R1 revised export diagnostics.

shafik added inline comments. Comment at: clang/lib/Sema/SemaModule.cpp:835 + + // C++20 [module.interface]p3: if (auto *ND = dyn_cast(D)) { Can you please add the quoted text for clarity. This is especially helpful if the text changes in the future and we

[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:4988 + if (SS->getCond()->containsErrors() || + !EvaluateDependentExpr(SS->getCond(), Info)) return ESR_Failed; yronglin wrote: > erichkeane wrote: > > It seems to me

[PATCH] D152632: [Clang] Add warnings for CWG2521

shafik added a comment. I am wondering why we don't fold this into `-Wreserved-identifier` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152632/new/ https://reviews.llvm.org/D152632 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D152632: [Clang] Add warnings for CWG2521

shafik added inline comments. Comment at: clang/include/clang/Basic/IdentifierTable.h:56 + NotStartsWithUnderscore, + ContainsDoubleUnderscore, +}; I would think starting with a double underscore would also not be allowed, at least that is my reading. CHANGE

[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:4989 if (SS->getCond()->isValueDependent()) { if (!EvaluateDependentExpr(SS->getCond(), Info)) return ESR_Failed; As far as I can tell `Value` will still not be set if w

[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); + Repository: rG LLVM Githu

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

shafik added inline comments. Comment at: clang/lib/AST/Decl.cpp:1831 + // Objective-C as an extension. + if (isa(this) && getLangOpts().ObjC) return true; It is not obvious to me if this is a drive by fix or this has a larger effect in the context of thi

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

shafik added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:18217 + NewFD->setIsPlaceholderVar(LangOpts.CPlusPlus && II && II->isPlaceholder()); if (PrevDecl && !isa(PrevDecl)) { Why can't we fold this into `FieldDecl::Create`? This comment applie

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

shafik added a comment. In D153296#612 , @erichkeane wrote: > So I think I'm pretty confident that the only time we would call > `EvaluateDependentExpr` is when we are in an error condition, so I'm > convinced the fix 'as is' is incorrect. The che

[PATCH] D105759: Implement P2361 Unevaluated string literals

shafik added inline comments. Comment at: clang/lib/Parse/ParseExpr.cpp:3268 + + return ParseStringLiteralExpression(false, true); +} Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105759/new/ https://reviews.llv

[PATCH] D153724: [clang] Fix a crash on invalid destructor

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. Make sense to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153724/new/ https://reviews.llvm.org/D153724 __

[PATCH] D105759: Implement P2361 Unevaluated string literals

shafik accepted this revision. shafik added a comment. LGTM but I don't see `asm` covered in the tests. Comment at: clang/lib/AST/Expr.cpp:1140 + case Unevaluated: +return sizeof(char); // Host; +break; Why not grouped w/ `Ordinary` above? ==

[PATCH] D153621: [Clang] Correctly handle $, @, and ` when represented as UCN

shafik added inline comments. Comment at: clang/test/Lexer/utf8-char-literal.cpp:22 char b = u8'\x80'; // ok -char c = u8'\u0080'; // expected-error {{universal character name refers to a control character}} +char c = u8'\u'; // ok char d = u8'\u1234'; // expected-error

[PATCH] D153649: [clang][Interp] Fix return statements with expresssion in void functions

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeStmtGen.cpp:312 return this->emitRet(*ReturnType, RS); +} else if (RE->getType()->isVoidType()) { + if (!this->visit(RE)) You could also guard the cleanup and `emitRet` above w

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

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeExprGen.cpp:1022 + +if (DiscardResult) + return this->emitPopPtr(E); Could you just pass `DiscardResult` to `visitLocalInitializer` CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D153536: [Clang] Implement P2169 A nice placeholder with no name

shafik added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:18217 + NewFD->setIsPlaceholderVar(LangOpts.CPlusPlus && II && II->isPlaceholder()); if (PrevDecl && !isa(PrevDecl)) { cor3ntin wrote: > shafik wrote: > > Why can't we fold this into `Fie

[PATCH] D153695: [clang][Interp] Fix passing parameters of composite type

shafik added inline comments. Comment at: clang/lib/AST/Interp/ByteCodeEmitter.h:71 - /// Parameter indices. - llvm::DenseMap Params; + /// Parameter indices. > + llvm::DenseMap Params; I don't understand the additional comment, Comment a

[PATCH] D153702: [Clang] Implement P2738R1 - constexpr cast from void*

shafik added inline comments. Comment at: clang/test/CXX/expr/expr.const/p5-26.cpp:13 +(void)static_cast(a); //cxx23-note {{cast from 'void *' is not allowed in a constant expression in C++ standards before C++2c}} +(void)static_cast(a); +(void)static_cast(a); --

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

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

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

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

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

shafik added a comment. What about template struct A { }; void foo() { new struct A {}; } I think this should be allowed, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153857/new/ https://reviews.llvm.org/D153857 ___

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

shafik added a comment. MSVC rejects the first but accepts the second which I think is wrong but worth testing: alignof(struct B {}); sizeof(struct B{}); https://godbolt.org/z/153jYqaPM Comment at: clang/include/clang/Parse/Parser.h:2359 case DeclSpecContext::DSC_te

[PATCH] D153954: [WIP][clang][analyzer] Relax alpha.cplusplus.EnumCastOutOfRange checker

shafik added a comment. I did not look at this in detail but I don't think this approach is correct. I fixed this for constant evaluation the other day and you can find the details here: D130058 The test suite I wrote should be sufficient for you to validate y

[PATCH] D153957: [C++20] [Modules] Allow Stmt::Profile to treat lambdas as equal conditionally

shafik added inline comments. Comment at: clang/lib/AST/StmtProfile.cpp:188 StmtProfilerWithoutPointers(llvm::FoldingSetNodeID &ID, ODRHash &Hash) -: StmtProfiler(ID, false), Hash(Hash) {} +: StmtProfiler(ID, false, false), Hash(Hash) {} ni

[PATCH] D153857: [clang] Fix new-expression with elaborated-type-specifier

shafik added a comment. In D153857#4455480 , @Fznamznon wrote: >> MSVC rejects the first but accepts the second which I think is wrong but >> worth testing: >> >> alignof(struct B {}); >> sizeof(struct B{}); > > These don't have `new` so they are not aff

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

shafik added a comment. I think I am going to land this as is and if we can come up w/ an example that covers the `annot_typename` I can do a follow-up. As this is now it fixes a crash bug. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134334/new/ https://reviews.llvm.org/D134334 ___

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

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd1fcce97a6af: [Clang] Fix crash in isCXXDeclarationSpecifier when

[PATCH] D154253: [clang] detect integer overflow through temporary values

shafik added a comment. Nice catch, I missed that in my last fix: https://reviews.llvm.org/D137897 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154253/new/ https://reviews.llvm.org/D154253 ___ cfe-commi

[PATCH] D154253: [clang] detect integer overflow through temporary values

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

[PATCH] D147073: [Coverage] Handle invalid end location of an expression/statement.

shafik added a comment. In D147073#4228290 , @zequanwu wrote: > I'm trying to add the test case from: > https://github.com/llvm/llvm-project/issues/45481#issuecomment-981028897, but > lit test fails in `` not found. You can just define your own `strong

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

shafik added a comment. gentle ping 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://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D144511: [Clang] Fix how ReadMacroParameterList handles comments in macro parameter-list when in -CC mode

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG20a3fb9740dd: [Clang] Fix how ReadMacroParameterList handles comments in macro parameter-list… (authored by shafik). Herald added a project: clang. H

[PATCH] D147037: [Clang][ICE] Corrected invalid invalid parameter index on some attributes with invalid indices applied to varargs functions

shafik added inline comments. Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3764-3767 if (!checkFunctionOrMethodParameterIndex(S, D, AL, 1, IdxExpr, Idx)) return; - + if (Idx.getASTIndex() >= getFunctionOrMethodNumParams(D)) +return; tbaeder wrote: > e

[PATCH] D147037: [Clang][ICE] Corrected invalid invalid parameter index on some attributes with invalid indices applied to varargs functions

shafik added a comment. For the benefit of those who have not seen the bug report: https://github.com/llvm/llvm-project/issues/61635 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147037/new/ https://reviews.llvm.org/D147037 __

[PATCH] D145852: [Clang][AST] Fix __has_unique_object_representations computation for unnamed bitfields.

shafik accepted this revision. shafik added a comment. LGTM, thank for adding the tests! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145852/new/ https://reviews.llvm.org/D145852 ___ cfe-commits mailing

[PATCH] D146234: [clang] Fix crash when handling nested immediate invocations

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

[PATCH] D147175: [clang] Add __is_trivially_equality_comparable

shafik added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:3315 Builder.Alignment, Builder.Alignment, CharUnits::Zero(), Builder.PrimaryBase, false, Builder.SharedVBPtrBase, + Builder.EndsWithZeroSizedObject, Builder.LeadsWithZe

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

shafik added a comment. In D146329#4207522 , @royjacobson wrote: > In D146329#4203174 , @shafik wrote: > >> I would have loved to test the case from >> https://github.com/llvm/llvm-project/issues/61335 directly

[PATCH] D147495: [Clang][Attributes] Add MeaningfulToClassTemplateDefinition to unavailable attribute

shafik created this revision. shafik added reviewers: rsmith, aaron.ballman, erichkeane. Herald added a subscriber: inglorion. Herald added a project: All. shafik requested review of this revision. There may be cases in which we want to diagnose a type as unavailable but it may not be complete at

[PATCH] D147531: [clang] Reset track of immediate function context when entering new function

shafik added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:15186 + // different handling. + ExprEvalContexts.back().InImmediateFunctionContext = FD->isConsteval(); + I am a bit concerned that we are updating `InImmediateFunctionContext` outside of `

[PATCH] D147530: [clang] Add test for CWG191

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

[PATCH] D147626: [clang] Do not crash when initializing union with flexible array member

shafik added a subscriber: rsmith. shafik added a comment. Thank you for this fix. Comment at: clang/lib/Sema/SemaInit.cpp:808 unsigned NumElems = numStructUnionElements(ILE->getType()); - if (RDecl->hasFlexibleArrayMember()) + if (!RDecl->isUnion() && RDecl->h

[PATCH] D147590: [clang] Add test for CWG607

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. Thank you for working on these DRs, LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147590/new/ https://reviews.llvm.org/D147590 ___

[PATCH] D147615: [clang][Sema][NFC] Save token name instead of the full token

shafik added inline comments. Comment at: clang/lib/Parse/ParseDeclCXX.cpp:962 + // Save the token name used for static assertion. + const char *TokName = Tok.getName(); It looks like this comes from a static array so it should be fine but let's see what Aar

[PATCH] D147717: [C++20][NFC] Claim full support for consteval again

shafik added a comment. In D147717#4248989 , @erichkeane wrote: > @cjdb has been running some GDB test suites against our compiler: I am > wondering if we could ask him to try the consteval ones too before we set > this? This is probably a good idea.

[PATCH] D147554: [clang] Mark CWG562 as N/A

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

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

shafik added a comment. It looks like this causes a regression, we can see in this bug report: https://github.com/llvm/llvm-project/issues/61851 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135370/new/ https://reviews.llvm.org/D135370 __

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

shafik added a comment. A quick test indicated the `!FunctionTemplate` is the culprit for the noted regression. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135370/new/ https://reviews.llvm.org/D135370 ___

[PATCH] D147495: [Clang][Attributes] Add MeaningfulToClassTemplateDefinition to unavailable attribute

shafik updated this revision to Diff 511526. shafik added a comment. - Add release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147495/new/ https://reviews.llvm.org/D147495 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/Attr.td clang/test/SemaCXX/attr-unavaila

[PATCH] D147495: [Clang][Attributes] Add MeaningfulToClassTemplateDefinition to unavailable attribute

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb206cde3504c: [Clang][Attributes] Add MeaningfulToClassTemplateDefinition to unavailable… (authored by shafik). Herald added a project: clang. Repos

[PATCH] D147580: [Clang][NFC] Refactor "Designators" to be more similar

shafik added a comment. I guess this is prep work for tackling unifying them later on as you attempted in https://reviews.llvm.org/D140584 I knew this is not exactly the same change but can you add the test case identified in https://github.com/llvm/llvm-project/issues/61118 and https://github

[PATCH] D147580: [Clang][NFC] Refactor "Designators" to be more similar

shafik added a comment. I guess I should have waited till I saw: https://reviews.llvm.org/D147673 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147580/new/ https://reviews.llvm.org/D147580 ___ cfe-commit

[PATCH] D147673: [Clang] Improve designated inits diagnostic location

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. Thank you for taking care of this, LGTM Comment at: clang/lib/Sema/SemaInit.cpp:2646 + if (Loc.isInvalid()) +// This could happen with a "null" designato

[PATCH] D147762: [Clang] Fix filtering of inline namespaces for friend functions

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane, troyj, rsmith, bruno. Herald added a project: All. shafik requested review of this revision. PR D135370 implemented a performance improvement but it restricted the filtering of decl

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

shafik added a comment. PR for fix: https://reviews.llvm.org/D147762 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135370/new/ https://reviews.llvm.org/D135370 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D147762: [Clang] Fix filtering of inline namespaces for friend functions

This revision was automatically updated to reflect the committed changes. Closed by commit rGd89c6530fdb5: [Clang] Fix filtering of inline namespaces for friend functions (authored by shafik). Herald added a project: clang. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D147782: [clang] Fix 'operator=' lookup in nested class

shafik added inline comments. Comment at: clang/lib/Sema/SemaLookup.cpp:1328 // namespace scope - SearchNamespaceScope = false; + SearchNamespaceScope = Name.getCXXOverloadedOperator() == OO_Equal; } Maybe we should also add

[PATCH] D147836: [clang] Add test for CWG1822

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

[PATCH] D147839: [clang] Add test for CWG2007

shafik added inline comments. Comment at: clang/test/CXX/drs/dr20xx.cpp:20 +// FIXME: the following code shouldn't instantiate A. +// int b = (&b2)->foo; +} It looks like gcc and MSVC also instantiate `A` for this case: https://godbolt.org/z/8W8eYoa38 I have to

[PATCH] D147839: [clang] Add test for CWG2007

shafik added inline comments. Comment at: clang/test/CXX/drs/dr20xx.cpp:20 +// FIXME: the following code shouldn't instantiate A. +// int b = (&b2)->foo; +} shafik wrote: > It looks like gcc and MSVC also instantiate `A` for this case: > https://godbolt.org/z/8W

[PATCH] D147848: [clang] Add test for CWG2370

shafik added inline comments. Comment at: clang/test/CXX/drs/dr23xx.cpp:182 + typedef N::type N_type; + // FIXME: `type` should be searched for in N + // friend void N::g(type); The implementation seems to all accept this example: https://godbolt.org/z/vE6bEP

[PATCH] D147904: [Clang] Fix cast to BigIntType in hasUniqueObjectRepresentations

shafik added a comment. Thank you for this quick fix Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147904/new/ https://reviews.llvm.org/D147904 ___ cfe-commits mailing list cfe-commits@lists.llvm.org htt

[PATCH] D147909: [clang] Implement CWG 2397

shafik added a comment. This looks good but I will let at least one more reviewer take a look Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147909/new/ https://reviews.llvm.org/D147909 ___ cfe-commits ma

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

shafik added inline comments. Comment at: clang/test/CXX/drs/dr3xx.cpp:1492 +// This is technically ill-formed; G is looked up in 'N::' and is not found. +// Rejecting this seems correct, but most compilers accept, so we do also. +f.N::F::~G(); // expected-error {{qu

[PATCH] D147839: [clang] Add test for CWG2007

shafik added inline comments. Comment at: clang/test/CXX/drs/dr20xx.cpp:20 +// FIXME: the following code shouldn't instantiate A. +// int b = (&b2)->foo; +} Endill wrote: > shafik wrote: > > shafik wrote: > > > It looks like gcc and MSVC also instantiate `A` for

[PATCH] D147848: [clang] Add test for CWG2370

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/CXX/drs/dr23xx.cpp:182 + typedef N::type N_type; + // FIXME: `type` should be searched for in N + // friend void N::g(type); E

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

shafik added a subscriber: aaron.ballman. shafik added inline comments. Comment at: clang/test/CXX/drs/dr3xx.cpp:1439 + +namespace dr399 { // dr399: 11 + // NB: reuse dr244 test Endill wrote: > Despite a couple of FIXME in CWG244 test (out of do

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

shafik added a comment. In D147920#4257369 , @Endill wrote: > I think I haven't stressed it enough, but this whole test is copied from > dr244, which is written by Richard. Understood, I appreciate the patience in explaining what I am missing. Sometime

[PATCH] D147839: [clang] Add test for CWG2007

shafik added a comment. In D147839#4257394 , @Endill wrote: > Replace unary `&` with `__builtin_addressof()`. It prevents unnecessary > template instantiation (presumably to find overloaded unary `&`), and make > Clang compliant since 3.4 Nice approach

[PATCH] D147839: [clang] Add test for CWG2007

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM, thank you again for all the effort in documenting these and adding tests, it is much appreciated. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm

[PATCH] D148029: [Clang] Fix crash caused by line splicing in doc comment

shafik added a comment. Do we have a test that covers valid doxygen comments? I was looking and I don't see an obvious one. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148029/new/ https://reviews.llvm.org/D148029 ___

[PATCH] D148029: [Clang] Fix crash caused by line splicing in doc comment

shafik added inline comments. Comment at: clang/lib/Sema/Sema.cpp:2404 break; +case RawComment::RCK_Invalid: + // FIXME: are there other scenarios that could produce an invalid I don't get why we don't have to handle the other `RawComment` cases h

[PATCH] D148029: [Clang] Fix crash caused by line splicing in doc comment

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

<    4   5   6   7   8   9   10   11   12   >