[PATCH] D81003: [clang] SequenceChecker: Also visit default arguments and default initializers.

riccibruno updated this revision to Diff 271659. riccibruno added a comment. - Remove a left-over FIXME in the tests. - Use a `%select` in the diagnostic. - Pack `AdditionalSourceLocationData` into two `unsigned`s. - Simplify the chain walking code in `checkUsage`. - Mark the patch as needing revi

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

riccibruno added a comment. In D82940#2124955 , @aaron.ballman wrote: > LGTM, thank you for the fix! Wait, I am not sure that it is the right fix. The body of the lambda should not be different from the body of the `CXXMethodDecl`. Repository: rG L

[PATCH] D82880: Fix PR35677: UB on __int128_t or __uint128_t template parameters.

riccibruno added inline comments. Comment at: clang/lib/AST/StmtPrinter.cpp:1159 + case BuiltinType::UInt128: +OS << "Ui128"; +break; `i128` and `Ui128` are not valid integer literal suffix. The output of `StmtPrinter` is intended to be valid C++. Unfor

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

riccibruno added a comment. > I'm not certain I understand, but I'm removing my LG while we figure it out. > Can you expound a bit further? Take a look at the AST dump of a small modification of the reduced test case which doesn't trigger the assertion: void should_not_crash_with_switch_in_l

[PATCH] D82954: Fix crash on overloaded postfix unary operators due to invalid sloc

riccibruno added inline comments. Comment at: clang/lib/AST/ExprCXX.cpp:82 + default: +return getNumArgs() == 1; + } This will be true for `operator->`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82954/new

[PATCH] D82880: Fix PR35677: UB on __int128_t or __uint128_t template parameters.

riccibruno added a subscriber: aaron.ballman. riccibruno added inline comments. Comment at: clang/lib/AST/StmtPrinter.cpp:1159 + case BuiltinType::UInt128: +OS << "Ui128"; +break; davidstone wrote: > riccibruno wrote: > > `i128` and `Ui128` are not valid

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

riccibruno added a comment. In D82940#2125939 , @martong wrote: > In D82940#2125183 , @riccibruno > wrote: > > > > I'm not certain I understand, but I'm removing my LG while we figure it > > > out. Can you expound

[PATCH] D82880: Fix PR35677: UB on __int128_t or __uint128_t template parameters.

riccibruno added a subscriber: majnemer. riccibruno added inline comments. Comment at: clang/lib/AST/StmtPrinter.cpp:1159 + case BuiltinType::UInt128: +OS << "Ui128"; +break; riccibruno wrote: > davidstone wrote: > > riccibruno wrote: > > > `i128` and `U

[PATCH] D83009: [clang][Serialization] Don't duplicate the body of LambdaExpr during deserialization

riccibruno created this revision. riccibruno added reviewers: vabridgers, martong, aaron.ballman. riccibruno added a project: clang. Herald added subscribers: cfe-commits, rnkovacs. 05843dc6ab97a00cbde7aa4f08bf3692eb83109d chan

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

riccibruno added a comment. > Yes, I think that my patch just traded-off a bug for another bug. What about > the following instead: > > Initially zero the `Stmt *` for the body, then when the body is needed > (either with `getBody` or with `children`) > populate it if needed from the call oper

[PATCH] D83009: [clang][Serialization] Don't duplicate the body of LambdaExpr during deserialization

This revision was automatically updated to reflect the committed changes. Closed by commit rGe4d178a75244: [clang][Serialization] Don't duplicate the body of LambdaExpr during… (authored by riccibruno). Changed prior to commit: https://reviews.llvm.org/D83009?vs=274951&id=275093#toc Repository

[PATCH] D83183: [clang] Rework how and when APValues are dumped

riccibruno created this revision. riccibruno added reviewers: aaron.ballman, steveire, ilya-biryukov, sammccall, martong. riccibruno added a project: clang. Herald added subscribers: cfe-commits, rnkovacs. Currently `APValue`s are dumped as a single string. This becomes quickly completely unread

[PATCH] D83183: [clang] Rework how and when APValues are dumped

riccibruno updated this revision to Diff 275783. riccibruno added a comment. Make `dumpAPValueChildren` a private member function and remove `TextNodeDumper::getOS`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83183/new/ https://reviews.llvm.org

[PATCH] D82442: [Coroutines] Warning if the return type of coroutine_handle::address is not void*

riccibruno added inline comments. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:10531 +def warn_coroutine_handle_address_invalid_return_type : Warning < + "return type of 'coroutine_handle<>::address should be 'void*' (have %0) in order to get capability with exi

[PATCH] D83183: [clang] Rework how and when APValues are dumped

riccibruno updated this revision to Diff 275713. riccibruno added a comment. Format the tests and make them more relevant. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83183/new/ https://reviews.llvm.org/D83183 Files: clang/include/clang/AST/AP

[PATCH] D83183: [clang] Rework how and when APValues are dumped

riccibruno marked 2 inline comments as done. riccibruno added a comment. Thanks for your comments! In D83183#2132975 , @aaron.ballman wrote: > Do none of the JSON tests break from this change? No, but only because I am not modifying the JSON output at

[PATCH] D83183: [clang] Rework how and when APValues are dumped

This revision was automatically updated to reflect the committed changes. Closed by commit rGf63e3ea558bb: [clang] Rework how and when APValues are dumped (authored by riccibruno). Changed prior to commit: https://reviews.llvm.org/D83183?vs=275783&id=275828#toc Repository: rG LLVM Github Mon

[PATCH] D83183: [clang] Rework how and when APValues are dumped

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rGf63e3ea558bb: [clang] Rework how and when APValues are dumped (authored by riccibruno). Changed prior to commit: https:

[PATCH] D81003: [clang] SequenceChecker: Also visit default arguments and default initializers.

riccibruno added a comment. @rsmith I have modified this patch to address your comment above (we should track where default argument(s) and/or default member initializer(s) were used). You accepted this originally but I would like to make sure that you are happy with the updated patch. I was a

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

riccibruno accepted this revision. riccibruno added a comment. This revision is now accepted and ready to land. This LGTM, with a few wording nits. > This patch fix clang crashes at using these functions in C and passing > incompatible structures as parameters in case of > __builtin_va_list/__b

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

This revision was automatically updated to reflect the committed changes. Closed by commit rG1ba6fb929396: [clang] Fix a crash when passing a C structure of incompatible type to a… (authored by ArcsinX, committed by riccibruno). Changed prior to commit: https://reviews.llvm.org/D82805?vs=274789

[PATCH] D83529: Summary: [clang] Provide a way for WhileStmt to report the location of its LParen and RParen.

riccibruno requested changes to this revision. riccibruno added inline comments. This revision now requires changes to proceed. Comment at: clang/lib/Serialization/ASTReaderStmt.cpp:275 + S->setLParenLoc(readSourceLocation()); + S->setRParenLoc(readSourceLocation()); } ---

[PATCH] D83529: Summary: [clang] Provide a way for WhileStmt to report the location of its LParen and RParen.

riccibruno accepted this revision. riccibruno added a comment. This revision is now accepted and ready to land. No objection from me, but I am not a reviewer. I am just accepting this to cancel my comment on the missing serialization. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D83681: [clang] Diagnose a misplaced lambda capture-default.

riccibruno created this revision. riccibruno added reviewers: rsmith, aaron.ballman. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. Currently a capture-default which is not the first element in the lambda-capture is diagnosed with a generic `expected variable name or '

[PATCH] D83438: [AST] Fix potential nullptr dereference in Expr::HasSideEffects

riccibruno added a comment. I think that adding an unittest for such a simple fix is a bit heavy handed. What about just exercising this code path. For example: void Test(int N) { int arr[N]; decltype([&arr]{}) *p; // expected-error {{lambda expression in an unevaluated operand}} }

[PATCH] D83681: [clang] Diagnose a misplaced lambda capture-default.

riccibruno updated this revision to Diff 277561. riccibruno added a comment. clang-format the tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83681/new/ https://reviews.llvm.org/D83681 Files: clang/include/clang/Basic/DiagnosticParseKinds.td

[PATCH] D83681: [clang] Provide a more specific diagnostic for a misplaced lambda capture-default.

riccibruno updated this revision to Diff 277987. riccibruno retitled this revision from "[clang] Diagnose a misplaced lambda capture-default." to "[clang] Provide a more specific diagnostic for a misplaced lambda capture-default.". riccibruno added a comment. Get rid of the note which doesn't ad

[PATCH] D83681: [clang] Provide a more specific diagnostic for a misplaced lambda capture-default.

riccibruno updated this revision to Diff 278150. riccibruno added a comment. clang-format again, sigh... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83681/new/ https://reviews.llvm.org/D83681 Files: clang/include/clang/Basic/DiagnosticParseKin

[PATCH] D83681: [clang] Provide a more specific diagnostic for a misplaced lambda capture-default.

riccibruno updated this revision to Diff 278164. riccibruno marked 3 inline comments as done. riccibruno added a comment. Add some tests showing that a by-ref capture is still parsed properly, including some tests with a capture containing a pack expansion. Repository: rG LLVM Github Monorepo

[PATCH] D83681: [clang] Provide a more specific diagnostic for a misplaced lambda capture-default.

riccibruno added inline comments. Comment at: clang/lib/Parse/ParseExprCXX.cpp:937 + return Invalid([&] { +Diag(Tok.getLocation(), diag::err_capture_default_first); + }); aaron.ballman wrote: > Would it make sense to provide a fix-it to move the

[PATCH] D83901: [clang] Disable a few formatting options for test/

riccibruno created this revision. riccibruno added reviewers: aaron.ballman, jdoerfert, lebedev.ri, MyDeveloperDay. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. Hopefully this will make the bot a little less noisy. Rationale for each: `AlignTrailingComments`: We don

[PATCH] D81787: [clang] Fix the serialization of LambdaExpr and the bogus mutation in LambdaExpr::getBody

This revision was automatically updated to reflect the committed changes. riccibruno marked an inline comment as done. Closed by commit rG05843dc6ab97: [clang] Fix the serialization of LambdaExpr and the bogus mutation in… (authored by riccibruno). Changed prior to commit: https://reviews.llvm.

[PATCH] D81786: [clang][utils] Modify make-ast-dump-check.sh to generate AST serialization dump tests

This revision was automatically updated to reflect the committed changes. Closed by commit rGc7350a3bab14: [clang][utils] Modify make-ast-dump-check.sh to generate AST serialization dump… (authored by riccibruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.

[PATCH] D81330: [clang] SequenceChecker: C++17 sequencing rule for overloaded operators.

This revision was automatically updated to reflect the committed changes. Closed by commit rGf5bbe390d23d: [clang] SequenceChecker: C++17 sequencing rule for overloaded operators. (authored by riccibruno). Changed prior to commit: https://reviews.llvm.org/D81330?vs=269012&id=272249#toc Reposit

[PATCH] D81869: Modify FPFeatures to use delta not absolute settings to solve PCH compatibility problems

riccibruno added inline comments. Comment at: clang/include/clang/AST/Stmt.h:620 +//unsigned FPFeatures : 21; +unsigned FPFeatures : 32; }; mibintc wrote: > This is a temporary change, I know you don't want me to change the assert to > allow a wider s

[PATCH] D82446: [clang] Fix duplicate warning

riccibruno added a comment. I apologise for a quick drive-by comment, but this is something I find very annoying when looking at the history of a piece of code: Could you spent a few minutes to make a more descriptive title and description? Fixes duplicate warning emitted by clang when char

[PATCH] D81003: [clang] SequenceChecker: Also visit default arguments and default initializers.

riccibruno added a comment. @rsmith > I think it's important that we handle this in the near future (though I don't > mind if you'd like to submit this patch as-is and deal with this as a > follow-on). For a case like: > > void f(int, int = a++); >// ... some time later ... >f(a); >

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

riccibruno added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:4539 +if (CXXRecordDecl *T2RecordDecl = +dyn_cast(T2RecordType->getDecl())) { + const auto &Conversions = T2RecordDecl->getVisibleConversionFunctions(); I cannot reproduce

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

riccibruno added inline comments. Comment at: clang/lib/Sema/SemaInit.cpp:4539 +if (CXXRecordDecl *T2RecordDecl = +dyn_cast(T2RecordType->getDecl())) { + const auto &Conversions = T2RecordDecl->getVisibleConversionFunctions(); ArcsinX wrote: > ri

[PATCH] D82805: [clang] Fix a crash on passing C structure of incompatible type to function with reference parameter

riccibruno added inline comments. Comment at: clang/test/Sema/init-ref-c.c:8 +} \ No newline at end of file ArcsinX wrote: > riccibruno wrote: > > I think it would be better to name this test `varargs-arm.c` since this bug > > can only happen with the varargs-r

[PATCH] D82940: [ASTReader][ASTWriter][PCH][Modules] Fix (de)serialization of SwitchCases

riccibruno added a comment. Let me take a look. Reduced test case: class C {} b; void should_not_crash_with_switch_in_lambda() { switch (1) default:; auto f = [] { switch (1) default:; }; } Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D81003: [clang] SequenceChecker: Also visit default arguments and default initializers.

riccibruno updated this revision to Diff 281999. riccibruno edited the summary of this revision. riccibruno added a comment. Rebased with a few clang-format fixes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81003/new/ https://reviews.llvm.org/D8

[PATCH] D85033: [clang] Provide a better name for unnamed parameters, lambda classes and lambda captures.

riccibruno created this revision. riccibruno added reviewers: erichkeane, rsmith. riccibruno added a project: clang. Herald added subscribers: cfe-commits, arphaman. riccibruno requested review of this revision. As a follow up to D84658 : For an unnamed parameter:

[PATCH] D84678: [clang] False line number in a function definition with "void" parameter

riccibruno added a comment. In D84678#2184687 , @Jac1494 wrote: > Hi @aaron.ballman , > Address your review comments. > Thank you for accepting this. I don't have commit access please commit this. > Thanks. As discussed with Aaron on IRC I can commit it

[PATCH] D84678: [clang] False line number in a function definition with "void" parameter

riccibruno added a comment. In D84678#2187747 , @riccibruno wrote: > In D84678#2184687 , @Jac1494 wrote: > >> Hi @aaron.ballman , >> Address your review comments. >> Thank you for accepting this. I don't have commi

[PATCH] D85033: [clang] Provide a better pretty-printed name for unnamed parameters, lambda classes and lambda captures.

riccibruno updated this revision to Diff 282296. riccibruno added a comment. Make the unit tests in `NamedDeclPrinterTest.cpp` more robust. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85033/new/ https://reviews.llvm.org/D85033 Files: clang/lib

[PATCH] D84678: [clang] False line number in a function definition with "void" parameter

This revision was automatically updated to reflect the committed changes. Closed by commit rG38d3e7533279: [clang] Use the location of the void parameters when complaining that only a… (authored by Jac1494, committed by riccibruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D85033: [clang] Provide a better pretty-printed name for unnamed parameters, lambda classes and lambda captures.

riccibruno updated this revision to Diff 282312. riccibruno added a comment. Add `-fno-delayed-template-parsing` to the new unit tests to also pass on Windows. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85033/new/ https://reviews.llvm.org/D8503

[PATCH] D85033: [clang] Provide a better pretty-printed name for unnamed parameters, lambda classes and lambda captures.

riccibruno updated this revision to Diff 282391. riccibruno edited the summary of this revision. riccibruno added a comment. Don't forget to increment the field iterator in the loop of `maybePrintFieldForLambdaCapture`, and modify the tests to test this. Repository: rG LLVM Github Monorepo C

[PATCH] D84599: [clang-index] Use NamedDecl::getDeclName() instead of NamedDecl::printName in USRGenerator::EmitDeclName

riccibruno added a comment. (Disclaimer: I am not at all familiar with this code) **A few notes in no particular order**: Some entities with special names should presumably be mangled, but are currently not. Example: namespace special_names { struct S { operator int(); }; int operator""_u

[PATCH] D83261: [OPENMP]Redesign of OMPExecutableDirective representation.

riccibruno added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:345 Diag(Loc, diag::err_omp_declare_mapper_wrong_var) -<< DMD->getVarName().getAsString(); +<< getOpenMPDeclareMapperVarName(); Diag(D->getLocation(), diag::note_entity_declared_at

[PATCH] D85144: [clang] Improve Dumping of APValues

riccibruno added a comment. Thanks for finishing this. I don't see why you are adding `dumpPretty`; the point of the `dump` function I added is to dump the `APValue` as the tree-like structure it is. But your `dumpPretty` doesn't do that at all. Instead it is just an alias for `printPretty`. S

[PATCH] D85144: [clang] Improve Dumping of APValues

riccibruno added inline comments. Comment at: clang/lib/AST/APValue.cpp:539 Out << '[' << Path[I].getAsArrayIndex() << ']'; -ElemTy = Ctx.getAsArrayType(ElemTy)->getElementType(); +ElemTy = cast(ElemTy.getCanonicalType())->getElementType(); } -

[PATCH] D85144: [clang] Improve Dumping of APValues

riccibruno added inline comments. Comment at: clang/include/clang/AST/PrettyPrinter.h:222 + /// Whether null pointers should be printed as nullptr or as NULL. + unsigned UseNullptr : 1; + riccibruno wrote: > This seems to be unrelated. And anyway shouldn't this

[PATCH] D85536: [clang] Add a matcher for template template parameters.

riccibruno created this revision. riccibruno added reviewers: aaron.ballman, klimek, njames93. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. riccibruno requested review of this revision. There are already matchers for type template parameters and non-type template par

[PATCH] D84599: [Index/USRGeneration] Use NamedDecl::getDeclName() instead of NamedDecl::printName in USRGenerator::EmitDeclName

riccibruno added a comment. ... or if a string such as `(unnamed struct at /path/to/input.cc:1:3)` is suitable in an USR then `printName` can be used. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D84599/new/ https://reviews.llvm.org/D84599 __

[PATCH] D85545: [Diagnostics] Diagnose missing comma in string array initialization

riccibruno added a comment. Is there a way to suppress this diagnostic if someone wants to legitimately initialize an element of the array with a long string by relying on string literal concatenation? Comment at: clang/lib/Sema/SemaExpr.cpp:6910 << InitArgList[I]->g

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

riccibruno added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp:1 -// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions +// RUN: %check_clang_tidy -std=c

[PATCH] D80514: [clang-tidy] modernize-use-trailing-return-type support for C++20 concepts and decltype

riccibruno added inline comments. Comment at: clang-tools-extra/test/clang-tidy/checkers/modernize-use-trailing-return-type.cpp:1 -// RUN: %check_clang_tidy -std=c++14,c++17 %s modernize-use-trailing-return-type %t -- -- -fdeclspec -fexceptions +// RUN: %check_clang_tidy -std=c

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

riccibruno created this revision. riccibruno added reviewers: rsmith, erichkeane, rjmccall. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. riccibruno requested review of this revision. Currently clang accepts a default argument containing a structured binding which is

[PATCH] D85612: [Sema] Use proper integral cast for an enumerate with a fixed bool type

riccibruno added inline comments. Comment at: clang/test/AST/ast-dump-enum-bool.cpp:1 +// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fsyntax-only -ast-dump=json %s | FileCheck %s + Why a (pretty unreadable) json test? There is also `make-ast-dump-check.sh`

[PATCH] D85144: [clang] Improve Dumping of APValues

riccibruno added a comment. I agree with you that it's fine to use `printPretty` for leaves (and additionally it would be annoying to duplicate the `LValue` case); that's what I was planning to do anyway. What I am not sure I agree with is the additional complexity to handle the (debugger-onl

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

riccibruno updated this revision to Diff 284364. riccibruno marked 2 inline comments as done. riccibruno edited the summary of this revision. riccibruno added a comment. Refer to the binding in the diagnostic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

riccibruno added inline comments. Comment at: clang/test/CXX/dcl.decl/dcl.meaning/dcl.fct.default/p7.cpp:51 + extern void h8a(int = sizeof(z)); // ok + extern void h8b(int = w); // expected-error {{default argument references local variable 'w'}} rsm

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

riccibruno updated this revision to Diff 284718. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85613/new/ https://reviews.llvm.org/D85613 Files: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/lib/Sema/SemaDeclCXX.cpp clang/test/CXX/dcl

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

riccibruno added inline comments. Comment at: clang/include/clang/AST/DeclCXX.h:3843 /// The declaration that this binding binds to part of. + // FIXME: Currently not set during deserialization of the BindingDecl; + // only set when the corresponding DecompositionDecl is vis

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

riccibruno added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:112 // if (DRE->isNonOdrUse() != NOUR_Unevaluated) return S.Diag(DRE->getBeginLoc(), This can use `DiagnoseIfOdrUse` as soon as the inconsistency between parameters and

[PATCH] D85536: [clang] Add a matcher for template template parameters.

This revision was automatically updated to reflect the committed changes. Closed by commit rGf4dccf115cc1: [clang] Add a matcher for template template parameters. (authored by riccibruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85536/new/ htt

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

riccibruno updated this revision to Diff 284803. riccibruno added a comment. Remove the now-unused `const VarDecl *` parameter to `DiagnoseIfOdrUse`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D85613/new/ https://reviews.llvm.org/D85613 Files:

[PATCH] D57747: [Sema] SequenceChecker: Fix handling of operator ||, && and ?:

riccibruno updated this revision to Diff 216156. riccibruno added a comment. Rebased Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57747/new/ https://reviews.llvm.org/D57747 Files: clang/lib/Sema/SemaChecking.cpp clang/test/Sema/warn-unsequenced.c clang/te

[PATCH] D58297: [Sema] SequenceChecker: C++17 sequencing rules for built-in operators <<, >>, .*, ->*, =, op=

riccibruno updated this revision to Diff 216157. riccibruno added a comment. Rebased Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58297/new/ https://reviews.llvm.org/D58297 Files: clang/lib/Sema/SemaChecking.cpp clang/test/CXX/drs/dr2xx.cpp clang/test/CXX

[PATCH] D58579: [Sema] SequenceChecker: C++17 sequencing rule for call expressions.

riccibruno updated this revision to Diff 216158. riccibruno retitled this revision from "[Sema] SequenceChecker: C++17 sequencing rule for call expression." to "[Sema] SequenceChecker: C++17 sequencing rule for call expressions.". riccibruno added a comment. Rebased Repository: rC Clang CHA

[PATCH] D66481: [C++20] Support for lambdas in unevaluated context

riccibruno added a comment. Can you submit the patch with the full context (ie: git diff -U999) ? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66481/new/ https://reviews.llvm.org/D66481 ___ cfe-commits mailing l

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

riccibruno added a comment. Is the test up-to-date ? I tried to apply the patch locally and I get a bunch of failures : error: 'warning' diagnostics expected but not seen: File /home/bruno/software/llvm-project/clang/test/SemaCXX/warn-xor-as-pow.cpp Line 73: result of '2 ^ 64' is 66; di

[PATCH] D66397: [Diagnostics] Improve -Wxor-used-as-pow

riccibruno added a comment. > This should not warn. Please verify that patch was applied correctly and you > use newly built clang with this patch. (I use arc patch DX) You were right, I messed up on my side. Sorry about the noise ! I don't have much to add to the macro yes/no discussion bu

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

riccibruno created this revision. riccibruno added reviewers: aaron.ballman, lebedev.ri. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. Since statements, expressions and types are allocated with the `BumpPtrAllocator` from `ASTContext` their destructor is not executed.

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

riccibruno added a comment. In D66646#1642708 , @lebedev.ri wrote: > SGTM, but i wonder if this should be done one level up, in `BumpPtrAllocator` > itself? I don't understand. `BumpPtrAllocator` is only used to allocate raw memory and doesn't know any

[PATCH] D66646: Ensure that statements, expressions and types are trivially destructible with a static_assert

riccibruno added a comment. > I was looking at SpecificBumpPtrAllocator, which knows it's type. > But if i look down the indirection, i think > AllocatorBase::Allocate()/AllocatorBase::Deallocate() is the place. I did look at that, but I think it won't work for two reasons: 1. AST nodes are ve

[PATCH] D66722: [clang] Ensure that comment classes are trivially destructible

riccibruno created this revision. riccibruno added reviewers: aaron.ballman, Mordante. riccibruno added a project: clang. Herald added a subscriber: cfe-commits. As in D66646 , these classes are also allocated with a `BumpPtrAllocator`, and therefore should be tri

[PATCH] D66722: [clang] Ensure that comment classes are trivially destructible

riccibruno updated this revision to Diff 217063. riccibruno added a comment. Also test that `DeclInfo` is trivially destructible. Thanks ! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66722/new/ https://reviews.llvm.org/D66722 Files: clang/lib/AST/Comment.cp

[PATCH] D66722: [clang] Ensure that comment classes are trivially destructible

This revision was automatically updated to reflect the committed changes. Closed by commit rG2535f04338c6: [clang] Ensure that comment classes are trivially destructible (authored by riccibruno). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66722/ne

[PATCH] D66564: [clang-tidy] new FPGA struct pack align check

riccibruno added a comment. I do not understand why this check is specific to FPGAs. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66564/new/ https://reviews.llvm.org/D66564 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D71920: [AST] Refactor propagation of dependency bits. NFC

riccibruno added inline comments. Comment at: clang/include/clang/AST/Expr.h:126 +if (TD) + D = D | DependencyFlags::Type; +if (VD) ilya-biryukov wrote: > Mordante wrote: > > Just curious why do you prefer `D = D | DependencyFlags::Type;` over `D |=

[PATCH] D87540: [clang-tidy] Fix false positive issue in performance-unnecessary-value-param for arguments being moved in the function body.

riccibruno added inline comments. Comment at: clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp:74 + // parmVarDecl picked up by this checker. It will be an empty string and will + // lead to an assertion failure when using hasName(std::string) being used

[PATCH] D87831: [clang] Expose helper function to turn PP keywords spelling into PPKeywordKind

riccibruno added inline comments. Comment at: clang/include/clang/Basic/TokenKinds.h:99 +PPKeywordKind getPPKeywordFromSpelling(const std::string &Name); + A string is expensive here and unneeded. Why not a `StringRef`? Comment at: clang/lib/

[PATCH] D84599: [Index/USRGeneration] Use NamedDecl::getDeclName() instead of NamedDecl::printName in USRGenerator::EmitDeclName

riccibruno added a comment. Friendly ping on this patch; the patches depending on this patch (D84658 and D85033 on Phab + others not uploaded yet) significantly improve the handling of unnamed entities in diagnostics. Reposit

[PATCH] D81003: [clang] SequenceChecker: Also visit default arguments and default initializers.

riccibruno added a comment. Friendly ping on this patch: this is the last change to `SequenceChecker` I had in mind. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81003/new/ https://reviews.llvm.org/D81003

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

riccibruno added a comment. > I don't know how could a `PredefinedExpression` lack the function name, > probably @riccibruno or @rjmccall can help with this - according to D53605 > . A `PredefinedExpr` whose declaration context is dependent has no name (see `Se

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

riccibruno added inline comments. Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21 + clang_analyzer_dump(__func__); + clang_analyzer_dump(__FUNCTION__); + clang_analyzer_dump(__PRETTY_FUNCTION__); + // expected-warning@-3 {{&Element{"func",0 S64b,char}}} + // e

[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

riccibruno added a comment. Is my comment on the deserialization of `BindingDecl`s in https://reviews.llvm.org/D85613?id=284364 related to this change? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86207/new/ https://reviews.llvm.org/D86207 _

[PATCH] D87004: [analyzer] Evaluate PredefinedExpressions

riccibruno added inline comments. Comment at: clang/test/Analysis/eval-predefined-exprs.cpp:7-21 + clang_analyzer_dump(__func__); + clang_analyzer_dump(__FUNCTION__); + clang_analyzer_dump(__PRETTY_FUNCTION__); + // expected-warning@-3 {{&Element{"func",0 S64b,char}}} + // e

[PATCH] D86207: (De-)serialize BindingDecls before DecompositionDecl

riccibruno added a comment. In D86207#2252409 , @aaronpuchert wrote: > In D86207#2251802 , @riccibruno > wrote: > >> Is my comment on the deserialization of `BindingDecl`s in >> https://reviews.llvm.org/D85613?id

[PATCH] D85613: [clang] Look through bindings when checking whether a default argument references a local entity.

riccibruno marked 2 inline comments as done. riccibruno added inline comments. Comment at: clang/lib/Sema/SemaDeclCXX.cpp:91 + auto CheckAndDiagnoseLocalEntity = [&](const VarDecl *VD, unsigned DiagID, + const auto &... DiagArgs) -> bool {

[PATCH] D80878: [clang] Prevent that Decl::dump on a CXXRecordDecl deserialises further declarations.

riccibruno added a comment. And what if deserialization is forced? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80878/new/ https://reviews.llvm.org/D80878 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D85960: [AST][FPEnv] Keep FP options in trailing storage of CastExpr

riccibruno added a comment. > This change allow a CallExpr to have optional FPOptionsOverride object, Should this be `CastExpr` instead? > stored in trailing storage. The implementaion is made similar to the way > used in CallExpr. Nit, but that's not really similar. In the `CallExpr` case the

[PATCH] D84599: [Index/USRGeneration] Use NamedDecl::getDeclName() instead of NamedDecl::printName in USRGenerator::EmitDeclName

riccibruno added a comment. To make this patch more acceptable I could also add a `Visit` function for `DecompositionDecl` and `MSGuidDecl` such that the current behaviour is preserved (I won't be able to test it though since these implicit AST nodes are not visited). Repository: rG LLVM Gi

[PATCH] D87278: [Ignore Expressions] Fix performance regression by inlining `Ignore*SingleStep`

riccibruno accepted this revision. riccibruno added a comment. Thanks! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87278/new/ https://reviews.llvm.org/D87278 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D69560: [clang-tidy] Add 'bugprone-easily-swappable-parameters' check

riccibruno added inline comments. Comment at: clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp:337 + if (Name.empty()) +Name = ""; + return Name; I think that this will become unreachable after D84658 and D85033. I may be missing a

[PATCH] D57747: [Sema] SequenceChecker: Fix handling of operator ||, && and ?:

riccibruno added a comment. In D57747#1774848 , @xbolva00 wrote: > Does the whole stack of patch need to be commited at once or maybe you can > land them individually? Hi, thanks for looking at this patch series ! If I remember correctly (it has been a

[PATCH] D57660: [Sema] SequenceChecker: Handle references, members and structured bindings.

riccibruno added a comment. In D57660#1764337 , @Mordante wrote: > I like this improvement. However I'm not a reviewer. Thanks for looking at the patch! > You can land some NFC changes in separate commit. Yep, indeed. Will do when I rebase it. Reposi

<    1   2   3   4   5   6   7   >