[PATCH] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added inline comments. Comment at: clang/include/clang/Analysis/Analyses/UnsafeBufferUsage.h:46 +/// The text indicating that the user needs to provide input there: +constexpr static const char *const UserFillPlaceHolder = "..."; } // end namespace clang

[PATCH] D141333: [-Wunsafe-buffer-usage][NFC] Refactor checkUnsafeBufferUsage

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG214312ef7ee4: [-Wunsafe-buffer-usage][NFC] Refactor checkUnsafeBufferUsage (authored by jkorous). Herald added a project: clang. Herald added a subsc

[PATCH] D141340: [-Wunsafe-buffer-usage] Use relevant source locations for warnings

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG39a63fc7fe98: [-Wunsafe-buffer-usage] Use relevant source locations for warnings (authored by jkorous). Herald added a project: clang. Herald added a

[PATCH] D141356: [-Wunsafe-buffer-usage] Group diagnostics by variable

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG237ca436adf4: [-Wunsafe-buffer-usage] Group diagnostics by variable (authored by jkorous). Herald added a project: clang. Herald added a subscriber:

[PATCH] D153059: [-Wunsafe-buffer-usage] Group parameter fix-its

jkorous added a comment. I vote for splitting the patch to make both the review and any future debugging or git archeology easier. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153059/new/ https://reviews.llvm.org/D153059 ___ cfe-commits mai

[PATCH] D156192: [-Wunsafe-buffer-usage] Stop generating incorrect fix-its for variable declarations with unsupported specifiers

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:1387 +// with the source range that is being replaced using fix-its. Especially when +// we often cannot obtain accruate source ranges of cv-qualified type +// specifiers. typo:

[PATCH] D137379: [-Wunsafe-buffer-usage] Add warnings for unsafe buffer accesses by array subscript operations

jkorous added inline comments. Comment at: clang/test/SemaCXX/warn-unsafe-buffer-usage.cpp:10-13 +void foo(...); + +void * bar(void); +char * baz(void); NoQ wrote: > ziqingluo-90 wrote: > > steakhal wrote: > > > NoQ wrote: > > > > ziqingluo-90 wrote: > > > > > st

[PATCH] D74746: [clang][doxygen] Fix -Wdocumentation warning for tag typedefs

This revision was automatically updated to reflect the committed changes. Closed by commit rG2f56789c8fe8: [clang][doxygen] Fix false -Wdocumentation warning for tag typedefs (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: h

[PATCH] D74746: [clang][doxygen] Fix -Wdocumentation warning for tag typedefs

jkorous added a comment. @vsapsai Sorry! I read your comment only after landing this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74746/new/ https://reviews.llvm.org/D74746 ___ cfe-commits mailing li

[PATCH] D75483: [Sema] Fix a crash when attaching comments to an implicit decl

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. Thanks a bunch for fixing this! That also means we can skip all the implicit declarations from the `for (const Decl *D : Decls) {` just a couple lines below your fix since implicit declarat

[PATCH] D69290: [clang] Report sanitizer blacklist as a dependency in cc1 instead of driver

This revision was automatically updated to reflect the committed changes. Closed by commit rG03b84e4f6d0e: [clang] Report sanitizer blacklist as a dependency in cc1 (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D69648: Add VFS support for sanitizers' blacklist' 2

This revision was automatically updated to reflect the committed changes. Closed by commit rG590f279c456b: [clang] Add VFS support for sanitizers' blacklists (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGE

[PATCH] D69648: Add VFS support for sanitizers' blacklist' 2

jkorous added a comment. Ahh, right. I originally wanted to support `-ivfsoverlay` in Driver as it seemed reasonable to check the existence of blacklist in Driver and open it in cc1 using the same fs. I got talked out of it and I didn't touch the Driver but seems like I should've replaced the c

[PATCH] D70440: [Driver] Use VFS to check if sanitizer blacklists exist

jkorous added inline comments. Comment at: clang/unittests/Driver/SanitizerArgsTest.cpp:100 + Contains(StrEq(std::string("-fsanitize-system-blacklist=") + + ASanBlacklist))); + // User blacklists should also be added. ily

[PATCH] D70556: clang/Modules: Refactor CompilerInstance::loadModule, NFC

jkorous added inline comments. Comment at: clang/include/clang/Lex/ModuleLoader.h:50 +// We failed to load the module, but we shouldn't cache the failure. +OtherUncachedFailure, }; Just a typo - the comma at the end. CHANGES SINCE LAST ACTION https

[PATCH] D10833: Retrieve BinaryOperator::getOpcode and BinaryOperator::getOpcodeStr via libclang and its python interface

jkorous added a comment. Hi @arthurp, I can review the libclang part of the patch. Could you please remove the changes that are just code formatting? You can land those as a separate NFC commit. Comment at: clang/tools/libclang/CIndex.cpp:259 + + std::pair Begin = SM.getDeco

[PATCH] D71029: [clangd] (take 2) Try harder to find a plausible `clang` as argv0, particularly on Mac.

jkorous added a comment. The use of `xcrun` looks sound to me. Comment at: clang-tools-extra/clangd/CompileCommands.cpp:53 + } + StringRef Path = Buf->get()->getBuffer().trim(); + if (Path.empty()) { The `trim()` is probably safe - just wondering if that cou

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

jkorous added a comment. We should either simplify the implementation to reflect that we don't support e. g. `*:42` (seems preferable to me) or have the codepaths that are currently not accessible through `-fverify` tested by other means. Comment at: clang/include/clang/Front

[PATCH] D74009: [clang] Improve diagnostic note for implicit conversions that are disallowed because they involve more than one user-defined conversion.

jkorous added a reviewer: jkorous. jkorous added a comment. Herald added a subscriber: dexonsmith. Hi @logan-5! I suggest you split the patch into two smaller ones so it is easier to review. 1. A NFC patch with refactoring of the interface (`bool` -> `UserDefinedConversionsKind`). 2. Patch that

[PATCH] D74371: [DirectoryWatcher] Fix misuse of FSEvents API and data race

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

[PATCH] D72883: [clangd][test] Disable a particular testcase in FindExplicitReferencesTest when LLVM_ENABLE_EXPENSIVE_CHECKS

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 rG42b3c38903c9: [clangd][test] Disable a particular testcase in FindExplicitReferencesTest when… (authored by jkorous). Hera

[PATCH] D72100: Allow matching "any file" in `VerifyDiagnosticConsumer`.

jkorous added inline comments. Comment at: clang/test/Frontend/verify-any-file.c:1 +// RUN: %clang_cc1 -verify %s 2>&1 + I feel that we should test the output with FileCheck to make it more robust. Seems like other tests for `-verify` do that. For example here: h

[PATCH] D44576: [clangd] Fix undefined behavior due to misaligned type cast

jkorous-apple added a comment. I commited the patch but forgot to quote this review. git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@327902 91177308-0d34-0410-b5e6-96231b3b80d8 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44576 __

[PATCH] D44575: [clangd][nfc] Give name to a magic constant

jkorous-apple added a comment. I commited the patch but forgot to quote this review. git-svn-id: https://llvm.org/svn/llvm-project/clang-tools-extra/trunk@327901 91177308-0d34-0410-b5e6-96231b3b80d8 Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44575 __

[PATCH] D42938: [Sema] Emit -Winteger-overflow for arguments in function calls, ObjC messages.

jkorous-apple accepted this revision. jkorous-apple added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D42938 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

jkorous-apple created this revision. jkorous-apple added reviewers: speziale.ettore, arphaman. Herald added a subscriber: cfe-commits. C++ [over.built]p4: For every pair (T, VQ), where T is an arithmetic type other than bool, and VQ is either volatile or empty, there exist candidate operator f

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

jkorous-apple updated this revision to Diff 141865. jkorous-apple added a comment. Added test for decrement being disabled for bool. Fixed comment in test (will put into separate NFC commit). https://reviews.llvm.org/D44988 Files: Sema/SemaOverload.cpp SemaCXX/overloaded-builtin-operators.c

[PATCH] D44988: [Sema] Fix decrement availability for built-in types

jkorous-apple added a comment. Spot on with the negative test idea! Should've done that myself. Thanks. https://reviews.llvm.org/D44988 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D39419: Fix crash when parsing objective-c++ containing invalid lambda

jkorous-apple created this revision. https://reviews.llvm.org/D39419 Files: lib/Parse/ParseExprCXX.cpp test/Parser/objcxx11-invalid-lambda.cpp Index: test/Parser/objcxx11-invalid-lambda.cpp === --- /dev/null +++ test/Parser/obj

[PATCH] D39419: Fix crash when parsing objective-c++ containing invalid lambda

jkorous-apple added inline comments. Comment at: lib/Parse/ParseExprCXX.cpp:1020 - PA.Commit(); - return false; + PA2.Revert(); + return true; arphaman wrote: > Just to clarify: It seems like this revert (in addition to two TPAs) is the > main addition. Di

[PATCH] D39419: Fix crash when parsing objective-c++ containing invalid lambda

jkorous-apple updated this revision to Diff 121448. jkorous-apple added a comment. newline at the end of test file https://reviews.llvm.org/D39419 Files: lib/Parse/ParseExprCXX.cpp test/Parser/objcxx11-invalid-lambda.cpp Index: test/Parser/objcxx11-invalid-lambda.cpp =

[PATCH] D39730: Enabling constructor code completion

jkorous-apple created this revision. Herald added a subscriber: eraman. It seems like constructor code completion was intentionally disabled ages ago in this commit (and refactored later): commit 33224e61bfca370850abae89bbd415a4dabe07fa Author: Douglas Gregor Date: Fri Sep 18 17:42:29 2009 +0

[PATCH] D39730: Enabling constructor code completion

jkorous-apple added a comment. I agree and it seems to be the case but I will upgrade my ad-hoc testing code snippet to a proper test as I should have done it in the first place. I also agree that this change looks really suspicious and I was basically surprised that it works. https://reviews

[PATCH] D39730: Enabling constructor code completion

jkorous-apple updated this revision to Diff 122470. jkorous-apple added a comment. I am able to enable ctor completion selectively in translate unit scope and namespace scope so ctor completion does not appear in dot access context. I haven't figured out how to explicitly specify that completion

[PATCH] D39730: Enabling constructor code completion

jkorous-apple added a comment. Sorry, what do you mean by "this works in the body of the class"? Definition of constructor inside class definition has been working even before: struct foo { foo(); f }; Do you mean that a test shall be added in order to check that it still works? https:/

[PATCH] D146669: [-Wunsafe-buffer-usage] Hide fixits/suggestions behind an extra flag, -fsafe-buffer-usage-suggestions.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM. Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:11792 "change type of '%0' to '%select{std::span|std::array|std::span::iterator}1' to preserve bounds

[PATCH] D143128: [-Wunsafe-buffer-usage] Fix-Its transforming `&DRE[any]` to `(DRE.data() + any)`

jkorous added a comment. This is an interesting topic. In the abstract I see the question as should the Fix-Its prioritize how the code will fit the desired end state (presumably modern idiomatic C++) or carefully respect the state of the code as is now. The only thing I feel pretty strongly ab

[PATCH] D145993: [-Wunsafe-buffer-usage] Reducing non-determinism in diagnostics output stream

jkorous accepted this revision. jkorous added a comment. +1 to moving the comparator to the .cpp file; otherwise LGTM! Thanks for fixing this! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145993/new/ https://reviews.llvm.org/D145993 _

[PATCH] D143128: [-Wunsafe-buffer-usage][WIP] Fix-Its transforming `&DRE[any]` to `(DRE.data() + any)`

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:162 + InnerMatcher)), + unless(callee(functionDecl(hasAttr(attr::UnsafeBufferUsage); + auto CastOperandMatcher = ziqingluo-90 wrote: > jkorous

[PATCH] D143455: [-Wunsafe-buffer-usage] Emit Fix-Its only for C++20 and later standards

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG8b6ae9bd7466: [-Wunsafe-buffer-usage] Emit Fix-Its only for C++20 and later standards (authored by jkorous). Herald added a project: clang. Herald ad

[PATCH] D143455: [-Wunsafe-buffer-usage] Emit Fix-Its only for C++20 and later standards

jkorous added a comment. We actually have only commits with warnings in `release/16.x` - no Fix-Its. So no need to cherry-pick. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143455/new/ https://reviews.llvm.org/D143455 ___

[PATCH] D137348: [-Wunsafe-buffer-usage] Introduce an abstraction for fixable code patterns.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137348/new/ https://reviews.llvm.org/D137348 ___ cfe-commits mailing list cfe-commits

[PATCH] D138253: [-Wunsafe-buffer-usage] NFC: Implement fix-strategies and variable-use-claiming.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138253/new/ https://reviews.llvm.org/D138253 ___ cfe-commits mailing list cfe-commits

[PATCH] D140179: [WIP][-Wunsafe-buffer-usage] Add unsafe buffer checking opt-out pragmas

jkorous added inline comments. Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:2379 +for (auto UnsafeUse : UnsafeUses) + if (!DE.isSafeBufferOptOut(SM, UnsafeUse->getBeginLoc())) +UnsafeUsesToReport.push_back(UnsafeUse); NoQ wrote: > I belie

[PATCH] D41897: Fixing a crash in Sema.

jkorous-apple added inline comments. Comment at: Sema/SemaDeclCXX.cpp:2426 +// Skip all dependent types in templates being used as base specifiers. +// Checks below assume that base specifier is a CXXRecord. +if (BaseType->isDependentType()) { aaron.b

[PATCH] D41897: Fixing a crash in Sema.

jkorous-apple added inline comments. Comment at: SemaCXX/base-class-ambiguity-check.cpp:1 +// RUN: %clang_cc1 -fsyntax-only %s + aaron.ballman wrote: > jkorous-apple wrote: > > aaron.ballman wrote: > > > jkorous-apple wrote: > > > > aaron.ballman wrote: > > > > >

[PATCH] D41897: Fixing a crash in Sema.

jkorous-apple updated this revision to Diff 129631. jkorous-apple added a comment. Fixed test based on Aaron's comments. https://reviews.llvm.org/D41897 Files: Sema/SemaDeclCXX.cpp SemaCXX/base-class-ambiguity-check.cpp Index: SemaCXX/base-class-ambiguity-check.cpp ===

[PATCH] D41897: Fixing a crash in Sema.

jkorous-apple added a comment. Thank you! https://reviews.llvm.org/D41897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D41897: Fixing a crash in Sema.

jkorous-apple added a comment. I do. Will commit this in a minute. https://reviews.llvm.org/D41897 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

jkorous-apple created this revision. https://reviews.llvm.org/D42170 Files: Parse/ParseTemplate.cpp Parser/typedef-instead-of-typename-typo.hpp clang/Basic/DiagnosticParseKinds.td Index: Parser/typedef-instead-of-typename-typo.hpp ==

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

jkorous-apple added inline comments. Comment at: Parse/ParseTemplate.cpp:492 + // Is there just a typo in the input code? ('typedef' instead of 'typename') + if (Tok.is(tok::kw_typedef)) { +Diag(Tok.getLocation(), diag::err_expected_template_parameter); vsa

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

jkorous-apple updated this revision to Diff 131041. jkorous-apple added a comment. Changes based on Volodymyr's comments. https://reviews.llvm.org/D42170 Files: FixIt/fixit-typedef-instead-of-typename-typo.cpp Parse/ParseTemplate.cpp clang/Basic/DiagnosticParseKinds.td Index: FixIt/fixi

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

jkorous-apple added a comment. I got rid of backslashes in test and added member of the type parameter with typo. Thanks for these suggestions. https://reviews.llvm.org/D42170 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llv

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

jkorous-apple updated this revision to Diff 131693. jkorous-apple added a comment. I got rid of backslashes in test and added member of the type parameter with typo. Thanks for suggestions. https://reviews.llvm.org/D42170 Files: FixIt/fixit-typedef-instead-of-typename-typo.cpp Parse/ParseT

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

jkorous-apple updated this revision to Diff 132135. jkorous-apple added a comment. Tried to make test more readable. https://reviews.llvm.org/D42170 Files: FixIt/fixit-typedef-instead-of-typename-typo.cpp Parse/ParseTemplate.cpp clang/Basic/DiagnosticParseKinds.td Index: FixIt/fixit-typ

[PATCH] D42170: Fixit for 'typedef' instead of 'typename' typo

jkorous-apple added a comment. Thanks for your patience with me still learning lit/filecheck. What do you think about the test now? I totally agree that this is subjective matter but I appreciate your feedback. https://reviews.llvm.org/D42170 ___

[PATCH] D117938: [clang][dataflow] Avoid MaxIterations overflow

This revision was automatically updated to reflect the committed changes. Closed by commit rGdd01d971aa2c: [clang][dataflow] Avoid MaxIterations overflow (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.o

[PATCH] D117238: [C2x] Add BITINT_MAXWIDTH support

jkorous added a comment. @aaron.ballman I believe this change broke the build starting with: https://green.lab.llvm.org/green/job/clang-stage1-cmake-RA-incremental/26915/ /Users/buildslave/jenkins/workspace/clang-stage1-cmake-RA-incremental/llvm-project/clang/include/clang/Basic/Diagnostic.h:

[PATCH] D117238: [C2x] Add BITINT_MAXWIDTH support

jkorous added a comment. I verified locally that reverting this patch fixes the build. Reverted in fad7e491a0770ac4336934030ac67d77e7af5520 to unblock Green Dragon, etc. @aaron.ballman Please take a look when you get a chance.

<    1   2   3   4   5