[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

lebedev.ri added a comment. I have looked through tests, and it is possible that i just missed it, but does it test/handle the cases like: struct S { int X, Y, Z; // <- should be diagnosed. } Comment at: clang-tidy/readability/IsolateDeclCheck.cpp:343 + auto Diag =

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

lebedev.ri added a comment. In https://reviews.llvm.org/D51949#1248861, @JonasToth wrote: > In https://reviews.llvm.org/D51949#1248850, @lebedev.ri wrote: > > > I have looked through tests, and it is possible that i just missed it, but > > does it test/handle the cases like: > > > > struct S {

[PATCH] D51949: [clang-tidy] new check 'readability-isolate-declaration'

lebedev.ri added a comment. Thank you for working on this! Tried on a pet project , Two hits: $ ninja [16/376] Checking validity of cameras.xml /home/lebedevri/rawspeed/data/cameras.xml validates [100/376] Building CXX object src/CMakeFiles/raw

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri created this revision. lebedev.ri added reviewers: JonasToth, aaron.ballman, alexfh, hokein, xazax.hun. lebedev.ri added a project: clang-tools-extra. Herald added subscribers: rnkovacs, mgorny. Detects when the integral literal or floating point (decimal or hexadecimal) literal has non

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri updated this revision to Diff 167547. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Move alias to after the new check, Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeL

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri added inline comments. Comment at: docs/ReleaseNotes.rst:96 +- New alias :doc:`cert-dcl16-c + ` to :doc:`readability-uppercase-literal-suffix Eugene.Zelenko wrote: > Please move alias after new checks. BTW, is there some tool to actually add this a

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri updated this revision to Diff 167552. lebedev.ri added a comment. - Deduplicate one test. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt clang-tidy/readability/CMakeLists.txt clang

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri added inline comments. Comment at: docs/ReleaseNotes.rst:96 +- New alias :doc:`cert-dcl16-c + ` to :doc:`readability-uppercase-literal-suffix JonasToth wrote: > lebedev.ri wrote: > > Eugene.Zelenko wrote: > > > Please move alias after new checks. >

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri created this revision. lebedev.ri added reviewers: rsmith, aaron.ballman, efriedma. clang has `-Wextra-semi` (https://reviews.llvm.org/D43162), which is not dictated by the currently selected standard. While that is great, there is at least one more source of need-less semis - 'null s

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri added inline comments. Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:33 +}; +constexpr llvm::StringLiteral IntegerLiteral::Name; +constexpr llvm::StringLiteral IntegerLiteral::SkipFirst; JonasToth wrote: > why are these declaration

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri updated this revision to Diff 167620. lebedev.ri marked 13 inline comments as done. lebedev.ri added a comment. Thank you for taking a look! - Rebased - Added an alias in `hicpp` module - Addressed //most// of the review comments. In https://reviews.llvm.org/D52670#1249898, @JonasToth

[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

lebedev.ri updated this revision to Diff 167621. lebedev.ri added a comment. As requested in https://reviews.llvm.org/D50902#1250110, actually document that `ICCK_IntegerTruncation` was only emitted by clang 7. Repository: rC Clang https://reviews.llvm.org/D50901 Files: docs/UndefinedBeha

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri marked 8 inline comments as done. lebedev.ri added inline comments. Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:165 + + diag(LiteralLocation, "%0 literal suffix '%1' is not upper-case") + << LiteralType::Name << S.OldSuffix

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri added inline comments. Comment at: clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp:53 + const auto *T = dyn_cast(Node.getType().getTypePtr()); + if (!T) +return false; JonasToth wrote: > lebedev.ri wrote: > > JonasToth wrote: > > > Maybe t

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri updated this revision to Diff 167658. lebedev.ri marked 16 inline comments as done. lebedev.ri added a comment. Herald added subscribers: Sanitizers, llvm-commits. Addressed remaining review notes: - Fixed dangling links in docs - Don't mishandle user-defined literals - Don't ignore ma

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri updated this revision to Diff 167661. lebedev.ri marked 5 inline comments as done. lebedev.ri added a comment. Address review notes. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52670 Files: clang-tidy/cert/CERTTidyModule.cpp clang-tidy/cert/CMakeLists.txt cl

[PATCH] D52670: [clang-tidy] Add new 'readability-uppercase-literal-suffix' check (CERT DCL16-C, MISRA C:2012, 7.3, MISRA C++:2008, 2-13-4)

lebedev.ri added inline comments. Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:660-678 - // Check if the range is entirely contained within a macro argument. - SourceLocation MacroArgExpansionStartForRangeBegin; - SourceLocation MacroArgExpansionStartForRangeEn

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

lebedev.ri added a comment. Thanks, that looks much better. Please clang-format the patch. This pattern is repeated at least 4 times now, i think some abstraction is needed. Comment at: clang-tidy/performance/ForRangeCopyCheck.cpp:49 + // Skip whitelisted types + const auto

[PATCH] D45776: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

lebedev.ri added a comment. Herald added a subscriber: arphaman. Did this intentionally omit the possibility to pass a group of prefixes, like `FileCheck` supports? Usefulness of this feature is somewhat penalized by this. Repository: rL LLVM https://reviews.llvm.org/D45776 __

[PATCH] D52691: [clang-tidy] NFC use CHECK-NOTES in tests for performance-move-constructor-init

lebedev.ri added inline comments. Comment at: test/clang-tidy/check_clang_tidy.py:172 '-check-prefix=' + check_notes_prefix, - '-implicit-check-not={{note|warning|error}}:'], + '-implicit-check-not=warning|error}}:|note: (?!FIX-IT)}}'],

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri added inline comments. Comment at: lib/Parse/ParseStmt.cpp:237 +SourceLocation SemiLocation = ConsumeToken(); +if (!HasLeadingEmptyMacro && getCurScope()->isCompoundStmtScope() && +!SemiLocation.isMacroID()) { rsmith wrote: > I'm a litt

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri updated this revision to Diff 168000. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. Thank you for taking a look! - Move it into `ParseCompoundStatementBody()`, thus fixing false-positives with `case X: ;` e.g. - Rename to `-Wextra-semi-stmt` - Add `-Wempty-i

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

lebedev.ri added a comment. @rsmith Ping. Though, https://reviews.llvm.org/D50901 is less controversial, so maybe best to start there.. Repository: rC Clang https://reviews.llvm.org/D50250 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

lebedev.ri added inline comments. Comment at: clang-tidy/performance/ForRangeCopyCheck.cpp:50 + const auto VarType = Var->getType(); + if (std::find_if(WhiteListTypes.begin(), WhiteListTypes.end(), + [&](llvm::StringRef WhiteListType) { JonasT

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri updated this revision to Diff 168017. lebedev.ri marked 3 inline comments as done. lebedev.ri added a comment. - Moved `-Wempty-init-stmt` into `-Wextra-semi-stmt` - Moved `-Wextra-semi-stmt` out of `-Wextra-semi` - Tentatively enabled `-Wextra-semi-stmt` in `-Wextra` (and removed `-We

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:167-168 + CXX11ExtraSemi, + ExtraSemiStmt, + EmptyInitStatement]>; -

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri updated this revision to Diff 168020. lebedev.ri added a comment. Slightly improved test coverage for macros in `extra-semi-resulting-in-nullstmt-in-init-statement.cpp` Repository: rC Clang https://reviews.llvm.org/D52695 Files: docs/ReleaseNotes.rst include/clang/Basic/Diagn

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri added inline comments. Comment at: include/clang/Basic/DiagnosticGroups.td:770 +NullPointerArithmetic, +ExtraSemiStmt ]>; I'm really unsure of this. Maybe this should only be `EmptyInitStatement`. Repository: rC Clang https://reviews.ll

[PATCH] D52771: [private][clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

lebedev.ri updated this revision to Diff 168088. lebedev.ri added a subscriber: cfe-commits. lebedev.ri added a comment. Finds classes that not only contain the data (non-static member variables), but also have logic (non-static member functions), and diagnoses all member variables that have any o

[PATCH] D45776: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

lebedev.ri added a comment. In https://reviews.llvm.org/D45776#1253934, @zinovy.nis wrote: > In https://reviews.llvm.org/D45776#1251256, @lebedev.ri wrote: > > > Did this intentionally omit the possibility to pass a group of prefixes, > > like `FileCheck` supports? > > Usefulness of this featur

[PATCH] D45776: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py

lebedev.ri added a comment. In https://reviews.llvm.org/D45776#1253934, @zinovy.nis wrote: > In https://reviews.llvm.org/D45776#1251256, @lebedev.ri wrote: > > > Did this intentionally omit the possibility to pass a group of prefixes, > > like `FileCheck` supports? > > Usefulness of this featur

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

lebedev.ri added inline comments. Comment at: test/Sema/ext_vector_casts.c:121 vf = d + vf; // expected-warning {{implicit conversion loses floating-point precision}} - vf = vf + 0x; + vf = vf + 0x; // expected-warning {{implicit conversion loses floating-p

[PATCH] D52835: [Diagnostics] Check integer to floating point number implicit conversions

lebedev.ri added inline comments. Comment at: test/Sema/ext_vector_casts.c:121 vf = d + vf; // expected-warning {{implicit conversion loses floating-point precision}} - vf = vf + 0x; + vf = vf + 0x; // expected-warning {{implicit conversion loses floating-p

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri updated this revision to Diff 168157. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. After looking at stage-2 of LLVM, remove `-Wextra-semi-stmt` from `-Wextra`, and add `-Wempty-init-stmt` back into `-Wextra`. Anything else? Repository: rC Clang https://

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

lebedev.ri updated this revision to Diff 168162. lebedev.ri marked 5 inline comments as done. lebedev.ri added a comment. Address review notes - more tests. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52771 Files: clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cp

[PATCH] D52771: [clang-tidy] Non-private member variables in classes (MISRA, CppCoreGuidelines, HICPP)

lebedev.ri added inline comments. Comment at: clang-tidy/misc/NonPrivateMemberVariablesInClassesCheck.cpp:26 + +AST_MATCHER(clang::CXXRecordDecl, hasNonStaticMethod) { + return hasMethod(unless(isStaticStorageClass())) JonasToth wrote: > I think this and the nex

[PATCH] D52695: [clang][Parse] Diagnose useless null statements (PR39111)

lebedev.ri updated this revision to Diff 168240. lebedev.ri marked an inline comment as done. lebedev.ri added a comment. Thank you for taking a look! - Reworded the diag as suggested a little - Do consume consequtive semis as one - Do still record each one of them in AST. I'm not sure if we wa

[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up

lebedev.ri added a comment. Needs a test. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52880 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52880: [clang-tidy] fix PR39167, bugprone-exception-escape hangs-up

lebedev.ri added a comment. In https://reviews.llvm.org/D52880#1255035, @JonasToth wrote: > In https://reviews.llvm.org/D52880#1255031, @lebedev.ri wrote: > > > Needs a test. > > > How shall i test it? It feels that this condition should have been there from > the beginning on, as the check only

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

lebedev.ri added a comment. Thanks looks even better, getting there. There are some spurious whiteline changes. Comment at: clang-tidy/performance/ForRangeCopyCheck.cpp:53 const auto *Var = Result.Nodes.getNodeAs("loopVar"); + // Ignore code in macros since we can't place

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

lebedev.ri added inline comments. Comment at: clang-tidy/utils/Matchers.cpp:20 +matchesAnyListedName(const std::vector &NameList) { + SmallString<256> NameRegEx; + llvm::raw_svector_ostream NameOut(NameRegEx); baloghadamsoftware wrote: > lebedev.ri wrote: > > `

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

lebedev.ri added inline comments. Comment at: clang-tidy/utils/Matchers.cpp:18-19 + +Matcher +matchesAnyListedName(const std::vector &NameList) { + SmallString<256> NameRegEx; Actually wait, what is this? It should be something like ``` AST_MATCHER_P(NamedDecl,

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

lebedev.ri added inline comments. Comment at: clang-tidy/utils/Matchers.cpp:1 +//===--- Matchers.cpp - clang-tidy-===// +// Also, why is this not in a header? I suspect that heavily pessimizes inlining. https://reviews.l

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

lebedev.ri added inline comments. Comment at: clang-tidy/utils/Matchers.cpp:18-19 + +Matcher +matchesAnyListedName(const std::vector &NameList) { + SmallString<256> NameRegEx; baloghadamsoftware wrote: > lebedev.ri wrote: > > Actually wait, what is this? > > It

[PATCH] D52892: [Clang-tidy: readability] readability check to convert numerical constants to std::numeric_limits

lebedev.ri added a comment. The check itself isn't in the diff. Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D52892 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52971: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py to support multiple prefixes

lebedev.ri added a comment. Thank you! Comment at: test/clang-tidy/check_clang_tidy.py:94-95 +for check in args.check_suffix: + if not re.match('^[A-Z0-9\-,]+$', check): +sys.exit('Only A..Z, 0..9, "," and "-" are ' + + 'allowed in check suffixes list,

[PATCH] D50250: [clang][ubsan] Implicit Conversion Sanitizer - integer sign change - clang part

lebedev.ri added a comment. Ping. Repository: rC Clang https://reviews.llvm.org/D50250 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

lebedev.ri added a comment. Ping! It seemed we were so close here :) Repository: rC Clang https://reviews.llvm.org/D50901 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50901: [clang][ubsan] Split Implicit Integer Truncation Sanitizer into unsigned and signed checks

lebedev.ri added a comment. In https://reviews.llvm.org/D50901#1257651, @filcab wrote: > Sorry about that. I’m away today but I don’t think you’ve answered my > questions about “why not support standalone UBSan in tests”. Sorry if I > missed the answers if you did. I did answer that question

[PATCH] D52727: [clang-tidy] White List Option for performance-unnecessary-value-param, performance-unnecessary-copy-initialization and performance-for-range-copy

lebedev.ri added a comment. That's it! Hopefully last portion of nits. Comment at: clang-tidy/performance/UnnecessaryCopyInitialization.cpp:65-66 has(varDecl(hasLocalStorage(), - hasType(matchers::isExpensiveToCopy()), +

[PATCH] D52892: [Clang-tidy] readability check to convert numerical constants to std::numeric_limits

lebedev.ri added inline comments. Comment at: clang-tidy/readability/NumericalCostantsToMaxIntCheck.h:25 +/// +class NumericalCostantsToMaxIntCheck : public ClangTidyCheck { +public: I feel like the name is overly vague. This *only* handles the cases of `-1` and

[PATCH] D52971: [clang-tidy] Customize FileCheck prefix in check_clang-tidy.py to support multiple prefixes

lebedev.ri added a comment. In https://reviews.llvm.org/D52971#1258086, @zinovy.nis wrote: > In https://reviews.llvm.org/D52971#1257702, @JonasToth wrote: > > > I think it would make sense to migrate one test already, to use the new > > capability > > > This change was inspired by @lebedev.ri, s

[PATCH] D52998: [benchmark] Disable exceptions in Microsoft STL

lebedev.ri added a comment. Are those warnings about C++ exceptions, or some windows-specific exception stuff? It seems the C++ exceptions are used in tests e.g. https://github.com/google/benchmark/search?q=throw&unscoped_q=throw https://github.com/google/benchmark/search?q=catch&unscoped_q=catch

[PATCH] D57571: [clang-tidy] A new OpenMP module

lebedev.ri created this revision. lebedev.ri added reviewers: JonasToth, aaron.ballman, alexfh, xazax.hun, hokein. lebedev.ri added projects: clang-tools-extra, OpenMP. Herald added subscribers: openmp-commits, arphaman, guansong, rnkovacs, mgorny. Herald added a project: clang. Just the empty ske

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new check

lebedev.ri updated this revision to Diff 184692. lebedev.ri retitled this revision from "[clang-tidy] openmp-use-default-none - a new module and a check" to "[clang-tidy] openmp-use-default-none - a new check". lebedev.ri edited the summary of this revision. lebedev.ri added a parent revision: D5

[PATCH] D57571: [clang-tidy] A new OpenMP module

lebedev.ri added a comment. In D57571#1380421 , @JonasToth wrote: > LGTM. In D57571#1380421 , @JonasToth wrote: > Committing both revision together would be good :) Yes. I can't land them since D57112

[PATCH] D57585: [AST][OpenMP] OpenMP Sections / Section constructs contain Structured blocks

lebedev.ri added a comment. In D57585#1380673 , @ABataev wrote: > LG Okay, that sounds promising, thank you for the review! Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57585/new/ https://reviews.llvm.org/D57585 ___

[PATCH] D57585: [AST][OpenMP] OpenMP Sections / Section constructs contain Structured blocks

lebedev.ri created this revision. lebedev.ri added reviewers: ABataev, OpenMP. lebedev.ri added projects: clang, OpenMP. Herald added subscribers: openmp-commits, guansong. I'm working on a clang-tidy check, much like existing bugprone-exception-escape

[PATCH] D57594: [AST][OpenMP] OpenMP single Construct contains Structured block

lebedev.ri created this revision. lebedev.ri added reviewers: ABataev, OpenMP. lebedev.ri added projects: clang, OpenMP. Herald added subscribers: openmp-commits, guansong. `OpenMP Application Programming Interface Version 5.0 November 2018`

[PATCH] D57615: [AST][OpenMP] OpenMP master Construct contains Structured block

lebedev.ri created this revision. lebedev.ri added reviewers: ABataev, OpenMP. lebedev.ri added projects: clang, OpenMP. Herald added subscribers: openmp-commits, guansong. Much like `single` construct (D57594 ): `OpenMP Application Programming Interface Version 5

[PATCH] D57615: [AST][OpenMP] OpenMP master Construct contains Structured block

lebedev.ri added a comment. @ABataev i'm not sure i have fully followed the https://bugs.llvm.org/show_bug.cgi?id=40563#c1 > The outlined function is not generated for the loop, so there is no problem > with the standard compatibility. Are you saying that in these cases of `master`, `critical`

[PATCH] D57615: [AST][OpenMP] OpenMP master Construct contains Structured block

lebedev.ri added a comment. In D57615#1381418 , @ABataev wrote: > In D57615#1381416 , @lebedev.ri > wrote: > > > @ABataev i'm not sure i have fully followed the > > https://bugs.llvm.org/show_bug.cgi?id=40563#c1 >

[PATCH] D57615: [AST][OpenMP] OpenMP master Construct contains Structured block

lebedev.ri added a comment. In D57615#1381428 , @ABataev wrote: > In D57615#1381427 , @lebedev.ri > wrote: > > > In D57615#1381418 , @ABataev wrote: > > > > > In D57615#1381

[PATCH] D57615: [AST][OpenMP] OpenMP master Construct contains Structured block

lebedev.ri abandoned this revision. lebedev.ri added a comment. Let's instead solve the problem of structured block not having an AST representation. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57615/new/ https://reviews.llvm.org/D57615

[PATCH] D57594: [AST][OpenMP] OpenMP single Construct contains Structured block

lebedev.ri abandoned this revision. lebedev.ri added a comment. Let's instead solve the problem of structured block not having an AST representation. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57594/new/ https://reviews.llvm.org/D57594

[PATCH] D57615: [AST][OpenMP] OpenMP master Construct contains Structured block

lebedev.ri added a comment. In D57615#1381447 , @ABataev wrote: > > Okay, enough is enough :) > > I think a very important phrase was repeated a number of times, and it > > highlights the **actual** problem here: > > > >> is not the representation of th

[PATCH] D57615: [AST][OpenMP] OpenMP master Construct contains Structured block

lebedev.ri added a comment. In D57615#1381461 , @ABataev wrote: > In D57615#1381443 , @lebedev.ri > wrote: > > > Let's instead solve the problem of structured block not having an AST > > representation. > > > Agai

[PATCH] D57655: clang-format with UseTab: Always sometimes doesn't insert the right amount of tabs.

lebedev.ri requested changes to this revision. lebedev.ri added a comment. This revision now requires changes to proceed. Test? Is this a fix, or a new formatting style? Comment at: WhitespaceManager.cpp:3-6 +// The LLVM Compiler Infrastructure // +// This

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

lebedev.ri added a comment. Herald added a project: clang. Ping. I'm not quite sure who is the best suited to review this.. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/ https://reviews.llvm.org/D57112 ___

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

lebedev.ri created this revision. lebedev.ri added reviewers: JonasToth, aaron.ballman. lebedev.ri added a project: clang-tools-extra. Herald added a subscriber: xazax.hun. Herald added a project: clang. The check should ignore the main function, the program entry point. It is not possible to use

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

lebedev.ri added inline comments. Comment at: test/clang-tidy/modernize-avoid-c-arrays.cpp:90 + +int not_main(int argc, char *argv[]) { + // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: do not declare C-style arrays, use std::array<> instead JonasToth wrote: > `ma

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

lebedev.ri added inline comments. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:35 + const clang::DeclContext *DC = Node.getDeclContext(); + const clang::FunctionDecl *FD = llvm::dyn_cast(DC); + if (!FD) JonasToth wrote: > There is `FunctionDecl->cast

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

lebedev.ri updated this revision to Diff 185492. lebedev.ri marked 4 inline comments as done. lebedev.ri added a comment. Address review notes, adjust docs. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57787/new/ https://reviews.llvm.org/D57787 Fi

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

lebedev.ri added inline comments. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:35 + const clang::DeclContext *DC = Node.getDeclContext(); + const clang::FunctionDecl *FD = llvm::dyn_cast(DC); + if (!FD) lebedev.ri wrote: > JonasToth wrote: > > There

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

lebedev.ri updated this revision to Diff 185499. lebedev.ri added a comment. Add an assert to ensure that `clang::FunctionDecl::castFromDeclContext()` is safe to do. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57787/new/ https://reviews.llvm.org/

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

lebedev.ri updated this revision to Diff 185587. lebedev.ri marked 4 inline comments as done and an inline comment as not done. lebedev.ri added a comment. And back to `dyn_cast()`. Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57787/new/ https://re

[PATCH] D57787: [clang-tidy] modernize-avoid-c-arrays: avoid main function (PR40604)

lebedev.ri added a comment. In D57787#1387406 , @aaron.ballman wrote: > LGTM aside from a nit. Thank you for the review. Comment at: clang-tidy/modernize/AvoidCArraysCheck.cpp:35 + const clang::DeclContext *DC = Node.getDeclContext(

[PATCH] D54978: Move the SMT API to LLVM

lebedev.ri added subscribers: mehdi_amini, lebedev.ri. lebedev.ri added a comment. Looks like this (all all other related Z3 reviews - D54975 , D54976 , D54977 ) review has completely omitted cfe-c

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

lebedev.ri added a comment. Exciting! Comment at: clang-tidy/utils/ExceptionAnalyzer.h:31-192 +enum class ExceptionState : std::int8_t { + Throwing,///< The function can definitly throw given an AST. + NotThrowing, ///< This function can not throw, given an AST. + Unknow

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

lebedev.ri added inline comments. Comment at: clang-tidy/utils/ExceptionAnalyzer.h:31-192 +enum class ExceptionState : std::int8_t { + Throwing,///< The function can definitly throw given an AST. + NotThrowing, ///< This function can not throw, given an AST. + Unknown,

[PATCH] D57896: Variable names rule

lebedev.ri added a comment. Pretty sure this patch should have gone to llvm-commits, not cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57896/new/ https://reviews.llvm.org/D57896 ___ cfe-co

[PATCH] D57896: Variable names rule

lebedev.ri added a comment. In D57896#1389046 , @michaelplatings wrote: > In D57896#1389042 , @lebedev.ri > wrote: > > > Pretty sure this patch should have gone to llvm-commits, not cfe-commits. > > > I just set t

[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

lebedev.ri added a comment. In D57915#1389549 , @TomTan wrote: > Added the tests back. Clang IR should not lower these to bswap calls because > they are global library functions. It might be slower to make the call to > library function than bswap, but t

[PATCH] D57915: [COFF, ARM64] Remove definitions for _byteswap library functions

lebedev.ri added a comment. In D57915#1389722 , @TomTan wrote: > In D57915#1389560 , @lebedev.ri > wrote: > > > In D57915#1389549 , @TomTan wrote: > > > > > Added the tests

[PATCH] D54399: Move ExprMutationAnalyzer to Tooling/Analysis (1/3)

lebedev.ri added a comment. In D54399#1391935 , @shuaiwang wrote: > Hi @rsmith what do you think of just get this in since Eric is not > responding? Or do you think I should run certain test to verify? I'm having a déjà vu. Wasn't there already an attem

[PATCH] D58016: fixes copy constructor generation of classes containing 0-length arrays followed by exactly 1 trivial field (fixes #40682)

lebedev.ri added a comment. Test? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58016/new/ https://reviews.llvm.org/D58016 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[PATCH] D58157: Stop enabling clang-tools-extra automatically when clang is in LLVM_ENABLE_PROJECTS

lebedev.ri added subscribers: cfe-commits, lebedev.ri. lebedev.ri added a comment. cfe-commits (if cfe-dev proper) should probably also be part of this disscussion. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58157/new/ https://reviews.llvm.org/D58157 _

[PATCH] D58178: isRawStringLiteral doesn't check all erroneous cases

lebedev.ri added a comment. 1. test? 2. please always upload all patches with full context (`-U9`) 3. The diff is malformed. `RawStringLiteralCheck.cpp` is not on the top level of the repo Repository: rCTE Clang Tools Extra CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58178/new/

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

lebedev.ri added a comment. In D58178#1396388 , @gmit wrote: > The added condition should be obvious as it is tested in the assert just > above. Well, then the fix is not correct. Because you won't get to that `return`, since the `assert()` just above w

[PATCH] D58178: [clang-tidy] RawStringLiteralCheck: isRawStringLiteral doesn't check all erroneous cases

lebedev.ri added a comment. In D58178#1396435 , @gmit wrote: > ? I'm sorry, but I disagree. > > Assert draws attention in the debug build only. > > In the release build asserts are not evaluated at all and the condition needs > to be checked in the code t

[PATCH] D58284: [clang] Switch to LLVM_ENABLE_IDE

lebedev.ri added a comment. In D58284#1399443 , @smeenai wrote: > CC @lebedev.ri, since I believe you raised some issues during the > corresponding LLVM change but those were addressed. Yeah, i don't think i have any comments here. The logic to pick the

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

lebedev.ri added a subscriber: alexfh. lebedev.ri added a comment. Herald added a subscriber: jdoerfert. Ping @hokein / @alexfh (as per git blame). Not sure who is best suited to review this. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D57112/new/ https://review

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Sorry for the reviews, i'm really stalling it seems.. This looks like a straight-forward change, but i'm not sure i'm familiar enough with ExceptionAnalyzer to fully properly review th

[PATCH] D57112: [ASTTypeTraits] OMPClause handling

lebedev.ri added a comment. Maybe ping @klimek then? :) In D57112#1400046 , @alexfh wrote: > In D57112#1399516 , @lebedev.ri > wrote: > > > Ping @hokein / @alexfh (as per git blame). > > Not sure who is best suit

[PATCH] D58035: [clang/DIVar] Emit flag for params that have unchanged values

lebedev.ri added inline comments. Comment at: lib/Sema/SemaExpr.cpp:11301 +EmitArgumentsValueModification(E); + SourceLocation OrigLoc = Loc; riccibruno wrote: > Comments: > > 1. Shouldn't you mark the variable to be modified only if > `CheckForModifiabl

[PATCH] D56933: [Tooling][RFC] Introduce Stencil library to simplify source code generation in refactorings.

lebedev.ri added subscribers: ilya-biryukov, gribozavr, lebedev.ri. lebedev.ri added a comment. See also: https://lists.llvm.org/pipermail/cfe-dev/2019-February/061414.html How are these two RFC's/API's supposed to interact/coexist/etc? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST A

[PATCH] D36836: [clang-tidy] Implement readability-function-cognitive-complexity check

lebedev.ri added a comment. Ping Repository: rL LLVM https://reviews.llvm.org/D36836 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

lebedev.ri added a comment. I did not test this yet, but looks better :) Comment at: test/clang-tidy/hicpp-exception-baseclass.cpp:26 } throw non_derived_exception(); // Bad + // CHECK-MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_excep

[PATCH] D37060: [clang-tidy] Improve hicpp-exception-baseclass to handle generic code better

lebedev.ri accepted this revision. lebedev.ri added a comment. This revision is now accepted and ready to land. Thank you for working on this! I just tried, and the original false-positive i was hitting is now gone. So as far i'm concerned, this is good to go. Comment at: test

[PATCH] D36892: [clang-tidy] check_clang_tidy.py: support CHECK-NOTES prefix

lebedev.ri updated this revision to Diff 113226. lebedev.ri added a comment. Rebased. Repository: rL LLVM https://reviews.llvm.org/D36892 Files: test/clang-tidy/check_clang_tidy.py Index: test/clang-tidy/check_clang_tidy.py

<    4   5   6   7   8   9   10   11   12   13   >