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

shafik updated this revision to Diff 521507. shafik added a comment. - Adding release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150226/new/ https://reviews.llvm.org/D150226 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/DiagnosticASTKinds.td clang/lib/AST/E

[PATCH] D149816: [clang][Interp] Implement __builtin_strcmp

shafik added inline comments. Comment at: clang/test/AST/Interp/builtin-functions.cpp:8 + + static_assert(__builtin_strcmp("abab", "abab") == 0); + static_assert(__builtin_strcmp("abab", "abba") == -1); Both empty strings `""` would be good as well. CHANGES S

[PATCH] D149816: [clang][Interp] Implement __builtin_strcmp

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

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

shafik added a comment. In D150226#4336755 , @rupprecht wrote: > We're still using `-Wno-enum-constexpr-conversion`, although I'm not sure if > we need that or if we just forgot to remove it after doing some cleanup. I'm > trying it out now. (Sorry, I'm

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

shafik added a comment. In D150226#4336771 , @manojgupta wrote: > We also use Wno-enum-constexpr-conversion in ChromeOS. There are many > packages that break with this warning. One of them is boost which is used in > many other packages. > > The errors

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

shafik added a comment. In D150226#4336803 , @thesamesam wrote: > Adding to the concerns raised above, I don't think we're there yet. See > https://bugs.gentoo.org/buglist.cgi?quicksearch=enum-constexpr-conversion&list_id=6843355 > and keep in mind tha

[PATCH] D150450: Add C++26 compile flags.

shafik added inline comments. Comment at: clang/lib/Driver/ToolChains/Clang.cpp:6649 .Case("c++20", "-std=c++20") // TODO add c++23 when MSVC supports it. + .Case("c++latest", "-std=c++26") --

[PATCH] D89046: [AST] Build recovery expression by default for all language.

shafik added a comment. Herald added a project: All. Hello folks, it looks like this PR is linked to a crash bug: https://github.com/llvm/llvm-project/issues/62711 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89046/new/ https://reviews.llvm.org/D

[PATCH] D150730: [Clang][Sema] Substitute constraints only for declarations with different lexical contexts

shafik added a comment. Can you please add context to the diff. thank you. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150730/new/ https://reviews.llvm.org/D150730 ___ cfe-commits mailing list cfe-comm

[PATCH] D150730: [Clang][Sema] Substitute constraints only for declarations with different lexical contexts

shafik added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:783 return true; - if (Old && New && Old != New) { + if (Old && New && Old != New && + Old->getLexicalDeclContext() != New->getLexicalDeclContext()) { erichkeane wrote: > Please

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

shafik accepted this revision. shafik added a comment. LGTM, apologies, it fell off my radar Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147920/new/ https://reviews.llvm.org/D147920 ___ cfe-commits mai

[PATCH] D150730: [Clang][Sema] Substitute constraints only for declarations with different lexical contexts

shafik added inline comments. Comment at: clang/lib/Sema/SemaConcept.cpp:783 return true; - if (Old && New && Old != New) { + if (Old && New && Old != New && + Old->getLexicalDeclContext() != New->getLexicalDeclContext()) { shafik wrote: > erichkeane

[PATCH] D150744: [NFC][CLANG] Fix uninitialized scalar field issues found by Coverity

shafik added inline comments. Comment at: clang/include/clang/Parse/Parser.h:1190 class ParseScopeFlags { Scope *CurScope; +unsigned OldFlags = 0; @tahonermann I feel like we should have a default member initializer for any member that by default is

[PATCH] D150875: Make dereferencing a void* a hard-error instead of warn-as-error

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

[PATCH] D144457: [clang][Interp] Handle global composite temporaries

shafik added a comment. I think it mostly makes sense but I want Aaron to also look at it. Comment at: clang/lib/AST/Interp/Pointer.cpp:10 #include "Pointer.h" +#include "Boolean.h" +#include "Context.h" Are all these headers really needed for the change below

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

shafik added a comment. In D150226#4353911 , @aaron.ballman wrote: > In D150226#4353905 , @dim wrote: > >> I submitted a similar workaround for the FreeBSD devel/gdb port, via >> https://bugs.freebsd.org/bugzill

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

shafik added a comment. In D150226#4354063 , @efriedma wrote: >> Also note, one of the bugs I reference shows how this breaks SFINAE: >> https://github.com/llvm/llvm-project/issues/57176 and that is not easily >> fixable. So this is non-conforming since

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

shafik added inline comments. Comment at: clang/test/CXX/drs/dr23xx.cpp:42 +namespace dr2335 { // dr2335: no drafting +// FIXME: all of the examples are well-formed. Endill wrote: > aaron.ballman wrote: > > Endill wrote: > > > shafik wrote: > > > > My comment o

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:9723 +// initialize this base/member. +CXXConstructorDecl *Constructor = +cast(S.CurContext); This and the changes below looks like unrelated formatting changes. R

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

shafik added a comment. I am mostly done but others should also look at this. Comment at: clang/lib/Sema/SemaInit.cpp:5348 + }; + ExprResult ER = CreateExprResult(); + if (InitExpr) Why not just use a conditional expression here or alternatively

[PATCH] D147743: [Clang][NFC] Rename methods/vars to reflect their real usage

shafik added inline comments. Comment at: clang/lib/Index/IndexBody.cpp:426 +return IndexCtx.handleReference(FD, D.getFieldLoc(), Parent, +ParentDC, SymbolRoleSet(), {}, E); + } nit Repository:

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

shafik added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:808 unsigned NumElems = numStructUnionElements(ILE->getType()); - if (RDecl->hasFlexibleArrayMember()) + if (!RDecl->isUnion() && RDecl->hasFlexibleArrayMember()) ++NumElems; -

[PATCH] D148712: [clang] Diagnose shadowing of lambda's template parameter by a capture

shafik added inline comments. Comment at: clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp:7 + // expected-note {{variable 'x' is explicitly captured here}} + auto h = [y = 0](y) { return 0; }; // expected-erro

[PATCH] D148712: [clang] Diagnose shadowing of lambda's template parameter by a capture

shafik added inline comments. Comment at: clang/test/CXX/expr/expr.prim/expr.prim.lambda/expr.prim.lambda.capture/p5.cpp:7 + // expected-note {{variable 'x' is explicitly captured here}} + auto h = [y = 0](y) { return 0; }; // expected-erro

[PATCH] D148712: [clang] Diagnose shadowing of lambda's template parameter by a capture

shafik added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:1381 +if (Capture.Id == TP->getIdentifier()) { + Diag(Capture.Loc, diag::err_template_param_shadow) << Capture.Id; + Diag(TP->getLocation(), diag::note_template_param_here);

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

shafik added a comment. 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] D147626: [clang] Reject flexible array member in a union in C++

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

[PATCH] D148802: [Sema] Lambdas are not part of immediate context for deduction

shafik added inline comments. Comment at: clang/test/CXX/temp/temp.deduct/p9.cpp:2 +// RUN: %clang_cc1 -std=c++20 -verify %s +template +auto f(T) -> decltype([]() { T::invalid; } ()); Maybe helpful to quote p9? Comment at: clang/test/CXX/temp

[PATCH] D148263: [clang] Mark CWG2009 as N/A

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM, from what I can tell this did not change anything. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148263/new/ https://reviews.llvm.org/D148

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

shafik added a comment. Do you know why these started timing out? I saw this locally the other day but could not figure out the root cause. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148851/new/ https://reviews.llvm.org/D148851 ___

cfe-commits@lists.llvm.org

shafik added inline comments. Comment at: clang/test/CXX/class/class.compare/class.compare.default/p1.cpp:18 bool operator<=(const A&) const = default; - bool operator==(const A&) const volatile && = default; // surprisingly, OK + bool operator==(const A&) const && = default

[PATCH] D148944: [clang][Driver] Fix crash with unsupported architectures in MinGW and CrossWindows.

shafik added a comment. Thank you for this fix. Can you please add a summary in the details explaining the motivation for this fix and a link to the github bug report as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148944/new/ https://revie

[PATCH] D149000: Update with warning message for comparison to NULL pointer

shafik added inline comments. Comment at: clang/test/Sema/warn-tautological-compare.c:102 +} \ No newline at end of file Please add back the missing newline Comment at: clang/test/SemaCXX/constant-expression-cxx2a.cpp:252 // ... but in the

[PATCH] D149003: [clang] Add test for CWG1821

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

[PATCH] D142401: [Clang] Fix a crash when recursively callig a default member initializer.

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

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

shafik accepted this revision. shafik added a comment. LGTM Comment at: clang/lib/Sema/SemaCXXScopeSpec.cpp:134 "specifier in SFINAE context?"); -if (!hasReachableDefinition(PartialSpec)) +if (PartialSpec->hasDef

[PATCH] D148712: [clang] Diagnose shadowing of lambda's template parameter by a capture

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

[PATCH] D149389: [clang] Fix default initializers being ignored when initializing templated aggregate types

shafik added a comment. So the change makes sense but the original issue: https://github.com/llvm/llvm-project/issues/62266 it worked for the first once but not the second, it also worked for the non-paren aggregate init. So I feel like the explanation is missing some details. It seems like th

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

shafik added a comment. I need to look at this more carefully but can we add a release note in the meantime Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148274/new/ https://reviews.llvm.org/D148274 ___

[PATCH] D149389: [clang] Fix default initializers being ignored when initializing templated aggregate types

shafik added a comment. Please also add a release note. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149389/new/ https://reviews.llvm.org/D149389 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D149276: [Clang] Fix parsing of `(auto(x))`.

shafik added a comment. I see that we do have an issue but so far the examples in the tests don't feel very natural and so I would like to understand if there is a larger motivation here. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149276/new/

[PATCH] D149436: [clang] Do not attempt to zero-extend _BitInt(1) when not required

shafik accepted this revision. shafik added a comment. LGTM Comment at: clang/test/CodeGen/ext-int.c:8 +unsigned _BitInt(1) GlobSize1 = 0; +// CHECK: @GlobSize1 = {{.*}}global i1 false + Interesting, it looks like `true` and `false` are valid for `i1` all the t

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

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

[PATCH] D149389: [clang] Fix default initializers being ignored when initializing templated aggregate types

shafik added a comment. In D149389#4306605 , @ayzhao wrote: > In D149389#4303966 , @shafik wrote: > >> So the change makes sense but the original issue: >> https://github.com/llvm/llvm-project/issues/62266 it wor

[PATCH] D149389: [clang] Fix default initializers being ignored when initializing templated aggregate types

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

[PATCH] D149514: Check if First argument in _builtin_assume_aligned_ is of pointer type

shafik added a comment. Thank you for the fix, you need to add a test that exercises the diagnostic you added, I think `clang/test/Sema/builtin-assume-aligned.c` would be the right place to add it. Also please add a release note in `clang/docs/ReleaseNotes.rst` CHANGES SINCE LAST ACTION ht

[PATCH] D149516: [Sema] `setInvalidDecl` for error deduction declaration

shafik added a comment. Thank you for the fix. I have some questions, why don't we always call `CheckDeductionGuideDeclarator` when calling `CXXDeductionGuideDecl::Create`, I felt like perhaps `CheckDeductionGuideDeclarator` should be rolled into `CXXDeductionGuideDecl::Create` but then I noti

[PATCH] D149637: [Clang] Correctly expand pack in binary subscript expression.

shafik accepted this revision. shafik added a comment. LGTM, other than my comment on the diagnostic wording Comment at: clang/test/SemaCXX/cxx2b-overloaded-operator.cpp:101 + int arr[] = {1, 2, 3}; + return arr[Is...]; // expected-error 2{{type 'int[3]' does not provide a s

[PATCH] D147840: [clang][Interp] Handle DiscardResult for DeclRef- and ParenExprs

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

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

shafik added a comment. 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] 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] D147621: [clang][Interp] Start handling mutable record members

shafik added a comment. Two cases to consider: https://godbolt.org/z/ovofPExGK namespace MutableFields { class Foo { public: constexpr Foo() : I(1) {} mutable int I; // ref-note {{declared here}} }; constexpr int foo() { constexpr Foo F; F.I = 12;

[PATCH] D148614: [clang][Interp] Add frame depth checking

shafik added a comment. The changes make sense, I am sure about any trade-offs of doing the checking during the call Vs doing at return. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148614/new/ https://reviews.llvm.org/D148614 ___ cfe-commi

[PATCH] D148982: [clang][Interp] Fix ignoring conditional operators

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

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

shafik added inline comments. Comment at: clang/test/AST/Interp/literals.cpp:783 + +namespace PredefinedExprs { + constexpr char heh(unsigned index) { Can we add tests for each predefined expressions, it does not look like there are a lot of them. Repository

[PATCH] D149612: [Sema] avoid merge error type

shafik added inline comments. Comment at: clang/lib/Sema/SemaDecl.cpp:4395 bool MergeTypeWithOld) { - if (New->isInvalidDecl() || Old->isInvalidDecl()) + if (New->isInvalidDecl() || Old->isInvalidDecl() || New->getType()->containsErrors() || Old->

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

shafik created this revision. shafik added reviewers: rsmith, aaron.ballman, erichkeane. Herald added a project: All. shafik requested review of this revision. Currently when using designated initializers in C++ we have a few extension. Two extension which are dangerous involved assigning to mult

[PATCH] D149187: [clang] Canonicalize system headers in dependency file when -canonical-prefixes

shafik added a comment. This commit came up in the following bug report: https://github.com/llvm/llvm-project/issues/62505 Please take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149187/new/ https://reviews.llvm.org/D149187 ___

[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField

shafik added inline comments. Comment at: clang/test/AST/Interp/records.cpp:500 namespace PointerArith { struct A {}; aaron.ballman wrote: > Neat! For the tests in this namespace, Clang and ICC agree, GCC and MSVC > agree, and users get to cry: https://god

[PATCH] D149612: [Sema] avoid merge error type

shafik added a subscriber: rsmith. shafik added a comment. In D149612#4314209 , @HerrCai0907 wrote: > in SemaType.cpp#BuildArrayType, It will generate `DependentSizedArrayType` > which cannot be merged. > So we can either add additional check in `Merge

[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField

shafik added a subscriber: rsmith. shafik added inline comments. Comment at: clang/test/AST/Interp/records.cpp:509 constexpr A *a2 = &b + 1; // expected-error {{must be initialized by a constant expression}} \ // expected-note {{cannot access ba

[PATCH] D152083: [clang] Warning for uninitialized elements in fixed-size arrays

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

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

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/Parser/cxx0x-for-range.cpp:67 +int a[] = {1, 2, 3, 4, 5}; +for (auto x = n ? 1 : 2 : a); // expected-error {{expected ';' in 'for' statem

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

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

cfe-commits@lists.llvm.org

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

[PATCH] D152285: Add support for the NO_COLOR environment variable

shafik added a comment. `CLICOLOR/CLICOLOR_FORCE` seems a lot more complicated, `NO-COLOR` feels like a win for low cost and does not preclude supporting others later on if we feel there is a need. So LGTM but I would like to see more feedback. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D152335: [Clang] Add check to Sema::AddAlignedAttr to verify active bits is not out of range

shafik created this revision. shafik added reviewers: aaron.ballman, erichkeane. Herald added a project: All. shafik requested review of this revision. If we provide too large a value for the alignment attribute `APInt::getZExtValue()` will assert. This PR adds a active bits check and folds it i

[PATCH] D152335: [Clang] Add check to Sema::AddAlignedAttr to verify active bits is not out of range

shafik updated this revision to Diff 529396. shafik marked 2 inline comments as done. shafik added a comment. - Switched to using APSInt::operator> as discussed offline w/ Erich and Aaron CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152335/new/ https://reviews.llvm.org/D152335 Files:

[PATCH] D152335: [Clang] Add check to Sema::AddAlignedAttr to verify active bits is not out of range

shafik marked an inline comment as done. shafik added inline comments. Comment at: clang/test/Sema/attr-aligned.c:1 -// RUN: %clang_cc1 -triple i386-apple-darwin9 -fsyntax-only -verify %s +// RUN: %clang_cc1 -triple x86_64-apple-darwin9 -fsyntax-only -verify %s

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/test/SemaTemplate/default-template-arguments.cpp:12 + auto lambda1 = [] {}; // expected-error {{default argument references local variable x_constexpr of enclosing function}} + auto lambda2 = [] {}; // expected-error {{default

[PATCH] D152335: [Clang] Add check to Sema::AddAlignedAttr to verify active bits is not out of range

shafik updated this revision to Diff 529714. shafik added a comment. - Add release note CHANGES SINCE LAST ACTION https://reviews.llvm.org/D152335/new/ https://reviews.llvm.org/D152335 Files: clang/docs/ReleaseNotes.rst clang/lib/Sema/SemaDeclAttr.cpp clang/test/Sema/attr-aligned.c I

[PATCH] D152335: [Clang] Add check to Sema::AddAlignedAttr to verify active bits is not out of range

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG086183c6c65f: [Clang] Add check to Sema::AddAlignedAttr to verify active bits is not out of… (authored by shafik). Herald added a project: clang. Ch

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

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

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

shafik added inline comments. Comment at: clang/test/SemaTemplate/aggregate-deduction-candidate.cpp:101 + + template struct E { +T t; I would also like to see this test: ``` template struct I { using type = T; }; template struct E { typename I::type

[PATCH] D152561: [AST] Always set dependent-type for the CallExpr for error-recovery in C.

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

[PATCH] D152570: [clang] Fix file mapping template arguments

shafik added inline comments. Comment at: clang/lib/AST/Expr.cpp:789 +class PrettyCallbacks final : public PrintingCallbacks { +public: This looks consistent with how other places that set `CallBacks` but I am not familiar with this area. ==

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

shafik added inline comments. Comment at: clang/include/clang/AST/DeclBase.h:1686 -/// [C++17] Only used by CXXDeductionGuideDecl. Indicates that -/// the Deduction Guide is the implicitly generated 'copy -/// deduction candidate' (is used during overload resolution

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

shafik added a comment. Looks like this change is coming up in: https://github.com/llvm/llvm-project/issues/63284 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D124534/new/ https://reviews.llvm.org/D124534 _

[PATCH] D139172: [clang] Mark CWG554 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/D139172/new/ https://reviews.llvm.org/D139172 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added a comment. It looks like we have an existing message for the regular function case e.g.: void g() { int x; void f(int y = x); auto lambda1 = [] {}; } see godbolt: https://godbolt.org/z/1vneM5b35 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added a comment. It looks like the existing diagnostic is issued by `CheckDefaultArgumentVisitor`. I think you can reuse it here but not 100% on that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139400/new/ https://reviews.llvm.org/D1394

[PATCH] D139429: [clang] Add test for CWG418

shafik added a comment. I think the relevant change from p1787 is to dcl.fct.default p4 , specifically: > ... Declarations that inhabit different scopes have completely distinct se

[PATCH] D139429: [clang] Add test for CWG418

shafik added a comment. In D139429#3974491 , @cor3ntin wrote: > I'm not sure how I feel about this. > Clang does not, implement correctly that paragraph at all, so I think the > best course here is to create an issue on the llvm github and mark the dr >

[PATCH] D139429: [clang] Add test for CWG418

shafik added a comment. In D139429#3974745 , @Endill wrote: >> I think the relevant change from p1787 is to dcl.fct.default p4, >> specifically: >> >>> ... Declarations that inhabit different scopes have completely distinct >>> sets of default arguments

[PATCH] D139429: [clang] Add test for CWG418

shafik added a subscriber: aaron.ballman. shafik added a comment. In D139429#3975028 , @Endill wrote: > I opened #59363 on bug > tracker. > What should I do about this patch then? I believe availability

[PATCH] D139429: [clang] Add test for CWG418

shafik added a comment. In D139429#3975223 , @Endill wrote: > @shafik does this imply that example from [over.match.best]/4 should be > included in this patch? Yes, I believe we should. We are only conforming if we get all of the cases correct. Repos

[PATCH] D139429: [clang] Add test for CWG418

shafik accepted this revision. shafik added a comment. This revision is now accepted and ready to land. LGTM, thank you for all your efforts. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139429/new/ https://reviews.llvm.org/D139429 ___ cfe-c

[PATCH] D139541: Fix parameter name in Sema::addInitCapture to ByRef.

shafik added inline comments. Comment at: clang/lib/Sema/SemaLambda.cpp:892-894 + LSI->addCapture(Var, /*isBlock*/ false, ByRef, /*isNested*/ false, Var->getLocation(), SourceLocation(), Var->getType(), /*Invalid*/ false); T

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:1594 + if (VarDecl *VD = dyn_cast(DRE->getDecl())) { +if (VD->isLocalVarDecl()) { + Diag(DRE->getLocation(), So if we look at `CheckDefaultArgumentVisitor::VistDeclRef

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/test/SemaTemplate/default-template-arguments.cpp:9 + + auto lambda1 = [] {}; // expected-error {{default argument references local variable x of enclosing function}} + auto lambda2 = [] {}; To clarify my commen

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:11 +#include // massberg #include "TreeTransform.h" Please remove. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139400/new/ https://re

[PATCH] D139705: [clang] fix zero-initialization fix-it for variable template

shafik added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:3870-3872 +} +if (const ASTTemplateArgumentListInfo *Info = VTSD->getTemplateArgsInfo()) + EndLoc = Info->getRAngleLoc(); If I understand correctly we either have a `VarTemplatePa

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

shafik added a comment. If I am reading the code correctly it looks like if the call to `(*I)->isValueDependent()` is true then the temporary will not be created and therefore we should not be attempting to access the slot. If this is the case then maybe the checking in `EvaluateWithSubstitutio

[PATCH] D139485: [LLVM] Remove redundant .c_str() and .get() calls where they are not needed.

shafik added inline comments. Comment at: lldb/source/API/SBPlatform.cpp:14 #include "lldb/API/SBLaunchInfo.h" -#include "lldb/API/SBPlatform.h" #include "lldb/API/SBUnixSignals.h" These redundant header removals looks unrelated and should be done as a separa

[PATCH] D139713: [Sema] Fix crash when evaluating nested call with value-dependent arg

shafik added a comment. In D139713#3989709 , @Pierre-vh wrote: > In D139713#3989071 , @shafik wrote: > >> If I am reading the code correctly it looks like if the call to >> `(*I)->isValueDependent()` is true then

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/lib/Sema/SemaTemplate.cpp:1594 + if (VarDecl *VD = dyn_cast(DRE->getDecl())) { +if (VD->isLocalVarDecl()) { + Diag(DRE->getLocation(), massberg wrote: > shafik wrote: > > So if we look at `Chec

[PATCH] D139400: [clang] Show error when a local variables is passed as default template parameter

shafik added inline comments. Comment at: clang/test/SemaTemplate/default-template-arguments.cpp:12 + auto lambda1 = [] {}; // expected-error {{default argument references local variable x_constexpr of enclosing function}} + auto lambda2 = [] {}; // expected-error {{default

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

shafik added inline comments. Comment at: clang/include/clang/AST/ExprCXX.h:4760 + : Expr(CXXParenListInitExprClass, T, + T->isLValueReferenceType() ? VK_LValue + : T->isRValueReferenceType() ? VK_XValue It looks like we use this id

[PATCH] D120489: [analyzer] Done some changes to detect Uninitialized read by the char array manipulation functions

shafik added a comment. Looks like it also broke lldb green dragon incremental as well: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/41865/consoleFull#-27141295349ba4694-19c4-4d7e-bec5-911270d8a58c Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.l

[PATCH] D121176: [ASTStructuralEquivalence] Add support for comparing ObjCCategoryDecl.

shafik added a comment. This make sense to me at a first pass, please make sure you run `check-lldb` as well. Comment at: clang/lib/AST/ASTStructuralEquivalence.cpp:1971 +return false; + unsigned SlotsToCheck = NumArgs > 0 ? NumArgs : 1; + for (unsigned I = 0; I < SlotsT

<    6   7   8   9   10   11   12   >