[PATCH] D133641: [Clang] [Sema] Ignore invalid multiversion function redeclarations

2022-09-10 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: tahonermann, erichkeane. Herald added a project: All. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. If a redeclaration of a multiversion function is invalid, it may be in a brok

[PATCH] D133641: [Clang] [Sema] Ignore invalid multiversion function redeclarations

2022-09-10 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. How the bug was working on this example: void foo() {} [[gnu::target("default")]] void foo() {} [[gnu::target("avx2")]] void foo() {} 1. Clang parses the definition of the `foo` function (line 1) 2. When parsing the second definition (line 2), Clang will delete `T

[PATCH] D133641: [Clang] [Sema] Ignore invalid multiversion function redeclarations

2022-09-13 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG67f08bf1bfa8: [Clang] [Sema] Ignore invalid multiversion function redeclarations (authored by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133641/n

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-14 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: clang-language-wg, aaron.ballman. Herald added a project: All. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Implements paper P2324R2 https://wg21.link/p2324r2 https://github.co

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-14 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. I thought it was a good idea to slightly rename `err_label_end_of_compound_statement` to `err_SWITCH_label_end_of_compound_statement` because we aren't going to support empty labels in switch statements. (GCC also doesn't compile it - https://godbolt.org/z/xabKaon8v) S

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

2022-09-14 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. @usaxena95, thanks! It was a really tough issue =) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119651/new/ https://reviews.llvm.org/D119651 ___ cfe-commits mailing list cfe-comm

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

2022-09-14 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 460209. Izaron added a comment. Add release note Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122078/new/ https://reviews.llvm.org/D122078 Files: clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cp

[PATCH] D122425: Pass -disable-llvm-passes to avoid running llvm passes

2022-03-26 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron accepted this revision. Izaron added a comment. This revision is now accepted and ready to land. Hi! I don't actually know much about llvm passes, but I see that the LLVM IR output is much more understandable now, thanks! It will look nicer when I rebase my patch D119792

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

2022-03-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 415904. Izaron added a comment. Herald added a project: All. Add the reverse_iterator-like pattern to tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119927/new/ https://reviews.llvm.org/D119927 Files: cl

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

2022-03-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. @Quuxplusone, thank you for the test idea! I added `test25` that resembles the pattern. Could you please look if we're okay with `test25`? If everything is OK, I could merge the patch myself: I've got repository merge access =) P. S. Sorry for the long delay, I was rea

[PATCH] D119470: [clang-tidy] Don't check decltype return types in `readability-const-return-type`

2022-03-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 416035. Izaron added a comment. Herald added a project: All. Don't warn on non-toplevel-const TypeOfType Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119470/new/ https://reviews.llvm.org/D119470 Files: clang

[PATCH] D119470: [clang-tidy] Don't check decltype return types in `readability-const-return-type`

2022-03-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron marked 3 inline comments as done. Izaron added a comment. > You've submitted some quality patches already, would you be interested in > obtaining commit privileges > (https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access)? Thanks! I obtained the commit access =) Now I don't

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

2022-03-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG3587b15abe68: [Clang] [P2025] More exhaustive tests for NRVO (authored by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119927/new/ https://reviews

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-03-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 416116. Izaron added a comment. Herald added a project: All. Rebased the patch on top of D119927 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119792/new/ https://reviews.llvm.

[PATCH] D119470: [clang-tidy] Don't check decltype return types in `readability-const-return-type`

2022-03-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG6043520c2072: [clang-tidy] Don't check decltype return types in `readability-const-return… (authored by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-03-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. In D119792#3389126 , @erichkeane wrote: > So P2025 has not successfully made it > through EWG, so this would have to be under a 'flag'. Also, I believe this > will end up being applied to C++23,

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-03-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. Let me explain a bit more =) The optimizations from this patch are allowed by the Standard (always have been allowed). So there is no need to restrict it under a flag or C++ version. @erichkeane your comment indeed would be applicable if I allowed NRVO for non-movable t

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

2022-03-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 416347. Izaron marked 7 inline comments as done. Izaron added a comment. Herald added a project: All. Fix issues Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118743/new/ https://reviews.llvm.org/D118743 Files:

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

2022-03-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang-tools-extra/clang-tidy/modernize/UseInlineConstVariablesInHeadersCheck.cpp:64-68 + unless(hasType(isVolatileQualified())), // non-volatile + unless(isTemplateVariable()), // non-template

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

2022-03-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. In D118743#3290591 , @LegalizeAdulthood wrote: > You've probably had this check in development for some time, but > we just updated the documentation for contributing to clang-tidy > and it might help to go over it for your new ch

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-03-18 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/include/clang/Sema/Scope.h:518 void addNRVOCandidate(VarDecl *VD) { +// every candidate except VD is "spoiled" now, remove them from the set ChuanqiXu wrote: > Firstly I am wondering why here doesn't check

[PATCH] D122075: [clang-tidy] Skip parentheses in `readability-make-member-function-const`

2022-03-19 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: mgehre-amd, njames93, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. Izaron requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. T

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

2022-03-19 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: hokein, njames93, aaron.ballman. Herald added subscribers: carlosgalvezp, xazax.hun. Herald added a project: All. Izaron requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. The c

[PATCH] D122075: [clang-tidy] Skip parentheses in `readability-make-member-function-const`

2022-03-21 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 416870. Izaron added a comment. Fixed as suggested by njames93. Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122075/new/ https://reviews.llvm.org/D122075 Files: clang-tools-extra/clang-tidy/readab

[PATCH] D122075: [clang-tidy] Skip parentheses in `readability-make-member-function-const`

2022-03-21 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGfc354d375232: [clang-tidy] Skip parentheses in `readability-make-member-function-const` (authored by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D1

[PATCH] D118922: [ASTMatchers] The `isInline` matcher now accepts inline variables

2022-02-03 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: klimek, aaron.ballman. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Inline variables is a feature from C++17. It makes sense to extend the `isInline` matcher in order to suppor

[PATCH] D118922: [ASTMatchers] The `isInline` matcher now accepts inline variables

2022-02-03 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. I am planning to reuse this matcher in my new checker (https://reviews.llvm.org/D118743, under development now), removing this line: AST_MATCHER(VarDecl, isVarInline) { return Node.isInline(); } //P.S. If this review is eventually approved, kindly please merge the comm

[PATCH] D118922: [ASTMatchers] The `isInline` matcher now accepts inline variables

2022-02-03 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/unittests/ASTMatchers/ASTMatchersInternalTest.cpp:195-201 -TEST(IsInlineMatcher, IsInline) { - EXPECT_TRUE(matches("void g(); inline void f();", - functionDecl(isInline(), hasName("f"; - EXPECT_TRUE(matche

[PATCH] D118922: [ASTMatchers] The `isInline` matcher now accepts inline variables

2022-02-04 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron abandoned this revision. Izaron added a comment. In D118922#3296542 , @njames93 wrote: > I had the same idea in D118900 Wow! That's a curious coincidence. I'm closing my PR =) Let it be your version as the earli

[PATCH] D118900: [ASTMatchers] Expand isInline matcher to VarDecl

2022-02-04 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. Thanks for the matcher! I'm planning to reuse it in my checker (D118743 ). Looking forward to seeing this commit merged. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118900/new/ https://r

[PATCH] D118900: [ASTMatchers] Expand isInline matcher to VarDecl

2022-02-04 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/docs/LibASTMatchersReference.html:5736 +MatcherVarDecl>isInline +Matches function and namespace declarations that are marked with +the inline keyword. >

[PATCH] D118900: [ASTMatchers] Expand isInline matcher to VarDecl

2022-02-04 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/docs/LibASTMatchersReference.html:5736 +MatcherVarDecl>isInline +Matches function and namespace declarations that are marked with +the inline keyword. Iz

[PATCH] D119095: [clang] Fix redundant functional cast in ConstantExpr

2022-02-06 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: aaron.ballman, cor3ntin. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When removing nested ConstantExprs, the tree transformer doesn't remove redundant CXXFunctionalCasts that

[PATCH] D119095: [clang] Fix redundant functional cast in ConstantExpr

2022-02-06 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. There are some godbolt links in the github issues. Expression `A{}.f();` generates the call of the move constructor from nowhere as well - https://godbolt.org/z/MceYedKzj With the patch this node (that lives inside an another `ConstantExpr` node): MemberExpr 0x5e4

[PATCH] D119098: [clang-tidy] Ignore variable template partial specializations in `misc-definitions-in-headers`

2022-02-06 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: njames93, hokein, LegalizeAdulthood. Herald added subscribers: carlosgalvezp, xazax.hun. Izaron requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Variable template partial spec

[PATCH] D119098: [clang-tidy] Ignore variable template partial specializations in `misc-definitions-in-headers`

2022-02-06 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. A copy-pasted message below, as usual =) //If this review is eventually approved, kindly please merge the commit on my behalf =) As I don't have merge access. My name is `Evgeny Shulgin` and email is `izaronpl...@gmail.com`. Sorry for inconvenience!// Repository: rG

[PATCH] D119375: [Clang][Sema] Do not evaluate value-dependent immediate invocations

2022-02-09 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: aaron.ballman, cor3ntin. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Value-dependent ConstantExprs are not meant to be evaluated. There is an assert in Expr::EvaluateAsConstan

[PATCH] D119375: [Clang][Sema] Do not evaluate value-dependent immediate invocations

2022-02-09 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. The assert that I mentioned in the summary: https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/ExprConstant.cpp#L14969 (Although `assert`s are deleted in release builds, one can check the violation by `llvm::errs() << isValueDependent() << "\n";` in the method)

[PATCH] D119375: [Clang][Sema] Do not evaluate value-dependent immediate invocations

2022-02-10 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 407449. Izaron added a comment. Fix comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119375/new/ https://reviews.llvm.org/D119375 Files: clang/lib/Sema/SemaExpr.cpp clang/test/SemaCXX/cxx2a-consteval.c

[PATCH] D119375: [Clang][Sema] Do not evaluate value-dependent immediate invocations

2022-02-10 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron marked an inline comment as done. Izaron added inline comments. Comment at: clang/lib/Sema/SemaExpr.cpp:16731 /*IsImmediateInvocation*/ true); - ExprEvalContexts.back().ImmediateInvocationCandidates.emplace_back(Res, 0); + /// Value-dependent constant expression a

[PATCH] D119375: [Clang][Sema] Do not evaluate value-dependent immediate invocations

2022-02-10 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 407564. Izaron added a comment. Add test comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119375/new/ https://reviews.llvm.org/D119375 Files: clang/lib/Sema/SemaExpr.cpp clang/test/SemaCXX/cxx2a-const

[PATCH] D119470: [clang-tidy] Don't check decltype return types in `readability-const-return-type`

2022-02-10 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: aaron.ballman, alexfh. Herald added subscribers: carlosgalvezp, xazax.hun. Izaron requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. The checker removes `const`s that are superf

[PATCH] D119470: [clang-tidy] Don't check decltype return types in `readability-const-return-type`

2022-02-10 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. I think we don't need to update the docs (https://clang.llvm.org/extra/clang-tidy/checks/readability-const-return-type.html) Because a user would expect this behaviour. //If this review is eventually approved, kindly please merge the commit on my behalf =) As I don't have

[PATCH] D119646: [clang] Allow consteval functions in default arguments

2022-02-12 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: aaron.ballman, erichkeane, andreasfertig. Herald added a subscriber: kristof.beyls. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We should not mark a function as "referenced" i

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

2022-02-12 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: aaron.ballman, erichkeane, rsmith. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. When instantiating a VarDecl initializing sub-AST, the evaluation context was only PotentiallyEv

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

2022-02-12 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 408220. Izaron added a comment. (Fast test comment 80 character line fix) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119651/new/ https://reviews.llvm.org/D119651 Files: clang/lib/Sema/SemaTemplateInstantia

[PATCH] D119646: [clang] Allow consteval functions in default arguments

2022-02-13 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. In D119646#3317473 , @cor3ntin wrote: > There is also https://reviews.llvm.org/D74130 which tries to address the same > issue in a very different way. > I don't really understand the different approaches yet. Thanks! I spent some

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

2022-02-13 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. > Should we maybe always treat `PotentiallyEvaluated` as `ConstantEvaluated` in > constant evaluated contexts? could that work ? Indeed, within this patch we can prevent similars bugs to appear. I researched other places where a new context is pushed, and haven't find ot

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-02-14 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: lebedev.ri, rsmith. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Before the patch we calculated the NRVO candidate looking at the variable's whole enclosing scope. The research

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-02-14 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. There is a nice proposal (P2025) Guaranteed copy elision for return variables by **Anton Zhilin**. The poll on the proposal showed that its ideas are very welcome: link to cplusplus/papers github

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-02-14 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. Cases that show the difference (they're covered in tests, though do we need an AST test as well?): X test(bool B) { if (B) { X y; // before: nrvo, after: nrvo (same) return y; } X x; // before: no nrvo, after: nrvo (better) return x; } X

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-02-15 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 408999. Izaron added a comment. Fix Scope::dumpImpl with more precise log Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119792/new/ https://reviews.llvm.org/D119792 Files: clang/include/clang/Sema/Scope.h c

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

2022-02-15 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 409035. Izaron added a comment. I've added an assert that will prevent similar bugs. Let's look if we're good with this one? Here is the list of eval contexts: https://github.com/llvm/llvm-project/blob/87b218b42b14e392aa0363a413d440b77bf04bd4/clang/include/cla

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-02-15 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. In D119792#3324354 , @Quuxplusone wrote: > I think it would be an extremely good idea to commit all 20 of these test > cases to clang/test/CodeGenCXX/, with their current behavior, as a > preliminary patch. Then, D119792

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

2022-02-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: mizvekov, Quuxplusone, sammccall, rsmith, doug.gregor. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is a preliminary patch ahead of D119792

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-02-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. This review will wait for D119927 to be merged, as it adds more tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D119792/new/ https://reviews.llvm.org/D119792 _

[PATCH] D119651: [clang] Fix evaluation context type for consteval function calls in instantiations

2022-02-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. After an investigation in GDB I can say that the assert seems to be wrong. Since Clang instantiates classes and functions "on the fly" where appropriate, we indeed can get a run-time evaluation context after a compile-time evaluation context. I was sure that evaluation c

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

2022-02-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 409374. Izaron added a comment. Thanks! Yes I should've write comments that are understandable not only for me =) I added comments to the existing tests as well. Though NRVO attribute is bound to the variable, I'm also more comfortable to place comments in th

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

2022-02-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 409389. Izaron added a comment. I placed links to corresponding p2025 examples. Some of the examples are reasonably absent from the test, such as 1st (RVO, not NRVO), 13th (consteval methods are not getting to LLVM IR), 17th (there are no NRVOs for non-class

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

2022-02-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron marked an inline comment as done. Izaron added inline comments. Comment at: clang/test/CodeGenCXX/nrvo.cpp:165 if (B) -return y; - return x; +return y; // NRVO is impossible + return x; // NRVO is impossible Quuxplusone wrote: > Technically,

[PATCH] D119927: [Clang] [P2025] More exhaustive tests for NRVO

2022-02-16 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron marked an inline comment as done. Izaron added a comment. @Quuxplusone Thanks for reviewing the patch! We can wait some time if someone else wants to take a look. Though I doubt if there may be major complaints on extendind the tests (especially with comments and references to a proposal)

[PATCH] D119375: [Clang][Sema] Do not evaluate value-dependent immediate invocations

2022-02-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. A friendly ping =) Seems like I don't have write access, so unfortunately I have to ask people to merge commits on my behalf. Let me copy-paste the usual comment of my reviews: //P.S. If this review is eventually approved, kindly please merge the commit on my behalf =)

[PATCH] D119792: [Clang] [P2025] Analyze only potential scopes for NRVO

2022-07-18 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. Hi! Unfortunately I don't have time to finish this pull request, so please feel free to take it and get it done =) (You may reuse the code from this PR or write a completely new implementation) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-19 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 469096. Izaron marked 4 inline comments as done. Izaron added a comment. Add time profiler test. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136022/new/ https://reviews.llvm.org/D136022 Files: clang/lib/AST

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-19 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. In D136022#3861245 , @jloser wrote: > I'd like to see some tests through before I approve. Thanks for the greenlight! I added a test that compiles a chunk of code and then checks the time trace graph in a human-readable form.

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-19 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. The test doesn't cover exactly all new traces though. For example I couldn't write a code that runs into the `EvaluateAsInt` method 🤔 If you have an idea for some funky constexpr code that can be tested, please write here =) Repository: rG LLVM Github Monorepo CHANG

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-19 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 469098. Izaron added a comment. Fix CMakeLists.txt Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136022/new/ https://reviews.llvm.org/D136022 Files: clang/lib/AST/ExprConstant.cpp clang/unittests/CMakeLists

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-20 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 469159. Izaron added a comment. Fix optionals with `value_or` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136022/new/ https://reviews.llvm.org/D136022 Files: clang/lib/AST/ExprConstant.cpp clang/unittests

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-20 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/unittests/Support/TimeProfilerTest.cpp:197-198 + + // NOTE: If this test is failing, run this test with + // `llvm::errs() << TraceGraph;` and change the assert above. +} aaron.ballman wrote: > This bit worries me

[PATCH] D136036: [Clang] Add __has_constexpr_builtin support

2022-10-21 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG5b567637e22b: [Clang] Add __has_constexpr_builtin support (authored by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION h

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-21 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 469771. Izaron added a comment. Mention this in the release notes. Thanks to Aaron for reviewing! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136022/new/ https://reviews.llvm.org/D136022 Files: clang/docs/R

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-21 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG27d8eedd5a3c: [clang] Add time profile for constant evaluation (authored by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTIO

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-21 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/unittests/Support/TimeProfilerTest.cpp:11 +#include "clang/Frontend/FrontendActions.h" +#include "clang/Lex/PreprocessorOptions.h" + thakis wrote: > Why is this in clang/unittests/Support (a new binary to boot)? Thi

[PATCH] D136022: [clang] Add time profile for constant evaluation

2022-10-22 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. In D136022#3877399 , @dyung wrote: > Hi @Izaron, several of our internal tests that run tests using `-ftime-trace` > started crashing when run which I bisected back to your change. I have filed > issue #58551

[PATCH] D136546: [clang][unittest] Resolve ClangSupportTest link time errors

2022-10-23 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. LGTM! I'm sorry that you had to fix CMakeLists.txt. It really "worked on my machine", I was running these commands: cmake --build build --target ClangSupportTests # build and link the binary ./build/tools/clang/unittests/Support/ClangSupportTests # run the binary Re

[PATCH] D136546: [clang][unittest] Resolve ClangSupportTest link time errors

2022-10-23 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGe5032d89e44a: [clang][unittest] Resolve ClangSupportTest link time errors (authored by jmciver, committed by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llv

[PATCH] D136549: [clang] Fix time profile in "isIntegerConstantExpr"

2022-10-23 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: dyung, aaron.ballman, jloser. Herald added a project: All. Izaron requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The time profiler in `Expr::isIntegerConstantExpr` used to call `Loc->pr

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

2022-10-23 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: jcranmer-intel, aaron.ballman, cor3ntin, efriedma. Herald added a subscriber: hiraditya. Herald added a project: All. Izaron requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commi

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

2022-10-23 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. This patch is similar to `__bultin_fmax`: https://reviews.llvm.org/D134369 The constexpr version of ilogb matches the libc realization, this is verified with the same tests: https://github.com/llvm/llvm-project/blob/main/libc/test/src/math/ILogbTest.h test_special_numb

[PATCH] D136549: [clang] Fix time profile in "isIntegerConstantExpr"

2022-10-23 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron marked 2 inline comments as done. Izaron added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:15908 - llvm::TimeTraceScope TimeScope("isIntegerConstantExpr", [&] { -return Loc->printToString(Ctx.getSourceManager()); - }); + ExprTimeTraceScope TimeScop

[PATCH] D136549: [clang] Fix time profile in "isIntegerConstantExpr"

2022-10-23 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG2bb50a55b0f5: [clang] Fix time profile in "isIntegerConstantExpr" (authored by Izaron). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136549/new/ https://re

[PATCH] D118743: [clang-tidy] Add `modernize-use-inline-const-variables-in-headers` check

2022-10-24 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. A friendly ping 🙂 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D118743/new/ https://reviews.llvm.org/D118743 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

2022-10-26 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St)) + return false; aaron.ballman wrote: > jcranmer-intel wrote: > > `long double` is `ppc_fp

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

2022-10-26 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 470962. Izaron added a comment. Add test for min/max value. Fix comment for ilog. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136568/new/ https://reviews.llvm.org/D136568 Files: clang/docs/LanguageExtension

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

2022-10-26 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron marked 2 inline comments as done. Izaron added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St)) + return false; jcranmer-intel wrote: > Izaron w

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

2022-10-27 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 471101. Izaron added a comment. Deal with MSVC where sizeof(long double) == sizeof(double) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136568/new/ https://reviews.llvm.org/D136568 Files: clang/docs/Language

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

2022-10-27 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St)) + return false; majnemer wrote: > Izaron wrote: > > jcranmer-intel wrote: > > > Izaron wr

[PATCH] D136568: [Clang] Support constexpr builtin ilogb

2022-10-27 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/lib/AST/ExprConstant.cpp:12452 +int Ilogb; +if (APFloat::opStatus St = ilogb(F, Ilogb); !isConstantOpStatus(St)) + return false; hubert.reinterpretcast wrote: > hubert.reinterpretcast wrote: > > hubert.

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 460976. Izaron added a comment. Created redundant-expression-cxx20.cpp Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122078/new/ https://reviews.llvm.org/D122078 Files: clang-tools-extra/clang-tidy/misc/Redun

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 460987. Izaron added a comment. Change C2x implementation status and C2x release notes Add an AST test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133887/new/ https://reviews.llvm.org/D133887 Files: clang/

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. > It should definitely be without warning in C23 mode and give an extension > warning in earlier modes. @aaron.ballman we have this extension warning for pre-C++23: def ext_label_end_of_compound_statement : ExtWarn< "label at end of compound statement is a C++2b ex

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 460990. Izaron added a comment. The new file was failing with message CHECK-FIXES, CHECK-MESSAGES or CHECK-NOTES not found in the input So I had to add an additional urrelevant check. I added a check for consteval function (there were no such tests previous

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 461002. Izaron added a comment. Add diagnostics for C language Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133887/new/ https://reviews.llvm.org/D133887 Files: clang/docs/ReleaseNotes.rst clang/include/cla

[PATCH] D122078: [clang-tidy] Ignore concepts in `misc-redundant-expression`

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 461003. Izaron added a comment. Fix test with `count 0`, thanks to @njames93 ! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122078/new/ https://reviews.llvm.org/D122078 Files: clang-tools-extra/clang-tidy/mi

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. > Why checking getLangOpts().C99 instead of just C There is no `getLangOpts().C`. Here are possible C/C++ opt flags: https://github.com/llvm/llvm-project/blob/7914e53e312074828293356f569d190ac6eae3bd/clang/include/clang/Basic/LangOptions.def#L86-L100 I have no understandin

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron updated this revision to Diff 461006. Izaron added a comment. Fix diagnostics for C. Thanks to @cor3ntin and @aaron.ballman! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D133887/new/ https://reviews.llvm.org/D133887 Files: clang/docs/Rele

[PATCH] D133887: [Clang] Support label at end of compound statement

2022-09-17 Thread Evgeny Shulgin via Phabricator via cfe-commits
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG510383626fe1: [Clang] Support label at end of compound statement (authored by Izaron). Changed prior to commit: https://reviews.llvm.org/D133887?v

[PATCH] D134136: [Clang] Support constexpr for builtin fmax, fmin, ilogb, logb, scalbn

2022-09-18 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron created this revision. Izaron added reviewers: cor3ntin, aaron.ballman. Herald added a subscriber: hiraditya. Herald added a project: All. Izaron requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. Support constexpr ver

[PATCH] D134136: [Clang] Support constexpr for builtin fmax, fmin, ilogb, logb, scalbn

2022-09-18 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added a comment. libc++ doesn't support constexpr-related patches from C++20 in ``: see issue https://github.com/llvm/llvm-project/issues/55370 I reviewed the code in `` and found out that since we use a dozen of math functions, we need to support more constexpr builtin math function. I

[PATCH] D134136: [Clang] Support constexpr for builtin fmax, fmin, ilogb, logb, scalbn

2022-09-18 Thread Evgeny Shulgin via Phabricator via cfe-commits
Izaron added inline comments. Comment at: clang/test/Sema/constant-builtins-math.cpp:16-17 + +// TODO: investigate why this is `INT_MIN + 1` instead of `INT_MIN` +static_assert(__builtin_ilogb(0.0) == -2147483647); +static_assert(__builtin_ilogb(__builtin_inf()) == 2147483647); -

  1   2   >