[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] 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] 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] 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] 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] 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] 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] 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] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous accepted this revision. jkorous added a comment. LGTM Thank you! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139737/new/ https://reviews.llvm.org/D139737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org

[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 = I am just wondering how does

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

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:703 + case Strategy::Kind::Span: +return FixItList{}; + case Strategy::Kind::Wontfix: jkorous wrote: > jkorous wrote: > > I am afraid I might have found one more

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

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:703 + case Strategy::Kind::Span: +return FixItList{}; + case Strategy::Kind::Wontfix: jkorous wrote: > I am afraid I might have found one more problem :( > I bel

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

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:703 + case Strategy::Kind::Span: +return FixItList{}; + case Strategy::Kind::Wontfix: I am afraid I might have found one more problem :( I believe that for `span

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

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:894 +if (VD->getType()->isPointerType()) + return fixVariableWithSpan(VD, Tracker, Ctx, Handler); +return {}; I believe we should add another condition here: `VD->isL

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

jkorous added a comment. I got a test failure in `SemaCXX/warn-unsafe-buffer-usage-fixits-local-var-span.cpp` which I believe is caused solely by the fact that we emit the diagnostics in different order. I am not sure it matters and since the Fix-Its clearly specify what source lines they apply

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

jkorous added a comment. I am sorry I haven't notice this earlier - let's fix this before we land the patch. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:690 + Val.toString(Txt, 10, true); + return Txt.data(); +} We either need a zero to terminate th

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

jkorous added inline comments. Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:127 +static auto isInUnspecifiedLvalueContext(internal::Matcher innerMatcher) { + auto isLvalueToRvalueCast = [](internal::Matcher M) { +return implicitCastExpr(hasCastKind(CastKind::CK_LVal

[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] 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] 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] 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] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added a comment. In D139737#4042093 , @NoQ wrote: > It sounds to me as if by landing the patch we'll temporarily make the > compiler emit incorrect fixes. I think we should avoid doing that. Is it > possible to wait until our first proof-of-conc

[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] D139737: [-Wunsafe-buffer-usage] Initiate Fix-it generation for local variable declarations

jkorous added a comment. Thanks for the rebase! Nit: I'd just replace `std::function` with `auto` as it saves us repeating the parameter types (and `#include `). Comment at: clang/lib/Analysis/UnsafeBufferUsage.cpp:617 + // Printers that print extent into OS and sets ExtKnow

[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] 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] 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] 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] D137346: -Wunsafe-buffer-usage: Initial commit - Transition away from raw buffer accesses.

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM. @aaron.ballman Do you have any objection to us landing this? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D137346/new/ https://reviews.llvm.org/D13734

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

jkorous added a comment. @aaron.ballman We'd like to start making progress on the implementation in parallel to iterating on the documentation and this is our first patch: https://reviews.llvm.org/D137346 Since we'll have about 4 people working full-time on this it isn't reasonable to expect yo

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

jkorous added inline comments. Comment at: clang/docs/SafeBuffers.rst:173 + #pragma unsafe_buffer_usage begin + +Such pragmas not only enable incremental adoption with much smaller granularity, aaron.ballman wrote: > jkorous wrote: > > aaron.ballman wrote: > >

[PATCH] D136811: -Wunsafe-buffer-usage: WIP: RFC: NFC: User documentation.

jkorous added a comment. Thank you for the feedback Gábor! Comment at: clang/docs/SafeBuffers.rst:36-37 +hardened mode where C++ classes such as ``std::vector`` and ``std::span``, +together with their respective ``iterator`` classes, are protected +at runtime agains

[PATCH] D136811: WIP: RFC: NFC: C++ Buffer Hardening user documentation.

jkorous added a comment. Thank you for the feedback Aaron! We really appreciate the effort you put into this! Comment at: clang/docs/SafeBuffers.rst:69-70 +containers such as ``std::span``, you can achieve bounds safety by +*simply writing good modern C++ code*. This is what t

[PATCH] D126908: [VerifyDiagnosticConsumer] Fix last line being discarded when parsing newline

jkorous added a comment. Thank you for the patch! Comment at: clang/test/SemaCXX/references.cpp:93 -struct C : B, A { }; // expected-warning {{direct base 'A' is inaccessible due to ambiguity:\nstruct C -> struct B -> struct A\nstruct C -> struct A}} +struct C : B, A { }

[PATCH] D133815: [analyzer] Support implicit parameters in path note

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG85d97aac80b8: [analyzer] Support implicit parameter 'self' in path note (authored by jkorous). Herald added a project: clang. Herald added a subscrib

[PATCH] D130055: Clang extensions yolo, woot & kaboom

jkorous added a comment. One more thing - the "attributes as a cross-TU metadata" idea might be (possibly with some squinting) conceptually similar to `enforce_tcb` attribute that @NoQ made me aware of some time ago. https://clang.llvm.org/docs/AttributeReference.html#enforce-tcb Repository:

[PATCH] D130055: Clang extensions yolo, woot & kaboom

jkorous added a comment. In D130055#3683279 , @beanz wrote: > I'm wondering if there could be a generalization of the attribute like: I can't imagine it is possible to design a reasonable and practically usable system of attributes to model anything but

[PATCH] D130055: Clang extensions yolo, woot & kaboom

jkorous added a comment. In D130055#3683173 , @aaron.ballman wrote: > Are there circumstances where we cannot "simply" infer this from the > constructor itself? (After instantiation) we know the class hierarchy, we > know the data members, and we know

[PATCH] D130055: Clang extensions yolo, woot & kaboom

jkorous added a comment. Hi Chris! This is a very interesting idea! I do have couple thoughts - mostly that this could lead to something great and I would love it to apply to as many relevant cases as possible. It looks like there is a possibility that a free function, static method or a metho

[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

jkorous added a comment. In D125349#3509073 , @aaron.ballman wrote: > It's interesting to note that `an_atomic_uint = an_atomic_uint + > an_enum_value` works correctly: https://godbolt.org/z/cvP9e6nh7. I was trying > to figure out whether the atomic qu

[PATCH] D125349: [Sema] Fix crash for C11 atomic in gnu++ mode

jkorous added a comment. In D125349#3509546 , @ahatanak wrote: > Is it not possible to handle this similarly to `volatile unsigned`? If I > replace `_Atomic unsigned` with `volatile unsigned`, I see > `LookupOverloadedBinOp` succeed without having to st

[PATCH] D123273: [utils] Avoid hardcoding metadata ids in update_cc_test_checks

jkorous added a comment. Reverting for now. Will take a look. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123273/new/ https://reviews.llvm.org/D123273 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D123273: [utils] Avoid hardcoding metadata ids in update_cc_test_checks

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGce583b14b2ec: [utils] Avoid hardcoding metadata ids in update_cc_test_checks (authored by jkorous). Herald added a project: clang. Herald added a sub

[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.

[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] 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] D113211: [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/

jkorous accepted this revision. jkorous added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/test/CodeGen/address-safety-attr.cpp:9 -// RUN: echo "fun:*BlacklistedFunction*" > %t.func.blacklist -// RUN: %clang_cc1 -std=c++11 -triple x86_64-

[PATCH] D113455: [clang][objc][codegen] Skip emitting ObjC category metadata when the category is empty

jkorous accepted this revision. jkorous added a comment. LGTM if we have test coverage for all the cases. Comment at: clang/test/CodeGenObjC/category-class-empty.m:1 +// RUN: %clang_cc1 -triple i386-apple-darwin9 -O3 -emit-llvm -o - %s | FileCheck %s +// PR7431 ---

[PATCH] D113211: [NFC][clang] Inclusive terms: replace uses of blacklist in clang/test/

jkorous requested changes to this revision. jkorous added a comment. This revision now requires changes to proceed. Hi! Thank you for the clean-up :) I feel there might be a bit of work still left. While renaming filenames and function names should be mostly inconsequential renaming command line

[PATCH] D97878: [DirectoryWatcher] Increase timeout to make test less flaky

jkorous added a comment. LGTM. Adding the comment would be great. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97878/new/ https://reviews.llvm.org/D97878 ___ cfe-commits mailing list cfe-commits@lists.

[PATCH] D93110: [analyzer] Implement a first version of suppressions via attributes

jkorous added a comment. Thanks for working on this! Comment at: clang/lib/StaticAnalyzer/Core/BugReporter.cpp:2907 +// start with it. +llvm::SmallVector Stack{DynTypedNode::create(*BugStmt)}; + Since IIUC a node can have multiple parents - does that me

[PATCH] D89959: UBSAN: emit distinctive traps in trapping mode

jkorous added a comment. Friendly ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89959/new/ https://reviews.llvm.org/D89959 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commit

[PATCH] D86230: [SourceManager] Skip module maps when searching files for macro arguments

This revision was automatically updated to reflect the committed changes. Closed by commit rGe7870223d8b5: [SourceManager] Skip module maps when searching files for macro arguments (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commi

[PATCH] D87176: [clang] Enable selectively turning on/off format-insufficient-args warning

jkorous added a comment. Had to fix the order of expected warnings in warning-wall.c 6fd8c69049a8 [clang] Update warning-wall.c test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org

[PATCH] D87176: [clang] Enable selectively turning on/off format-insufficient-args warning

This revision was automatically updated to reflect the committed changes. Closed by commit rG1e86d637eb4f: [clang] Selectively ena/disa-ble format-insufficient-args warning (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github M

[PATCH] D88133: [Analyzer][WebKit] Use tri-state types for relevant predicates

jkorous added a comment. Created this for eventual post-commit review. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D88133/new/ https://reviews.llvm.org/D88133 ___ cfe-commits mailing list cfe-commits@li

[PATCH] D88133: [Analyzer][WebKit] Use tri-state types for relevant predicates

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 rG47e6851423fd: [Analyzer][WebKit] Use tri-state types for relevant predicates (authored by jkorous). Herald added a project

[PATCH] D86231: [SourceManager] Explicitly check for potential iterator underflow

This revision was automatically updated to reflect the committed changes. Closed by commit rGae726fecae9a: [SourceManager] Explicitly check for potential iterator underflow (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github M

[PATCH] D83814: [clangd] Add Random Forest runtime for code completion.

jkorous added subscribers: dexonsmith, akyrtzi. jkorous added a comment. Hi @usaxena95 and @sammccall, I am wondering about couple high-level things. Do you guys intend to open-source also the training part of the model pipeline or publish a model trained on generic-enough training set so it co

[PATCH] D86992: [libclang] Expose Rewriter in libclang API

This revision was automatically updated to reflect the committed changes. Closed by commit rG69e5abb57b70: [libclang] Add CXRewriter to libclang API (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D8

[PATCH] D86991: [libclang] Expose couple AST details

This revision was automatically updated to reflect the committed changes. Closed by commit rG052f83890349: [libclang] Expose couple more AST details via cursors (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHA

[PATCH] D66854: [index-while-building] PathIndexer

This revision was automatically updated to reflect the committed changes. Closed by commit rG4da126c3748f: [index-while-building] PathIndexer (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llvm.org/D66854?vs

[PATCH] D82837: [Analyzer][WebKit] UncountedLambdaCaptureChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rG820e8d8656ec: [Analyzer][WebKit] UncountedLambdaCaptureChecker (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES

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

jkorous added a comment. Sounds like a good idea. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83438/new/ https://reviews.llvm.org/D83438 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https:

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

This revision was automatically updated to reflect the committed changes. Closed by commit rGfdb69539bcd2: [AST] Fix potential nullptr dereference in Expr::HasSideEffects (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Mon

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

This revision was automatically updated to reflect the committed changes. Closed by commit rG6e089e98a9d5: [libclang] Fix crash when visiting a captured VLA (authored by ckandeler, committed by jkorous). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

jkorous added a comment. @ckandeler do you have commit access or do you want me to land the patch? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82629/new/ https://reviews.llvm.org/D82629 ___ cfe-commi

[PATCH] D82740: [libclang]: check validity before visiting Stmt node

jkorous added a comment. @milianw I just approved https://reviews.llvm.org/D82629 - I feel like that patch is addressing the core issue. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82740/new/ https://reviews.llvm.org/D82740 __

[PATCH] D82629: [libclang] Fix crash when visiting a captured VLA.

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

[PATCH] D82740: [libclang]: check validity before visiting Stmt node

jkorous added a comment. Your patch definitely fixes the symptoms of the bug. I just want to make sure that we aren't covering some logic error with a bandaid as it would be harder to find out the root cause once we land this. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D82740: [libclang]: check validity before visiting Stmt node

jkorous added a comment. @milianw could you try to reduce the reproducer you have? Maybe take lldb and see where does the `nullptr` come from? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D82740/new/ https://reviews.llvm.org/D82740

[PATCH] D77179: [Analyzer][WebKit] UncountedCallArgsChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rGa7eb3692e762: [Analyzer][WebKit] UncountedCallArgsChecker (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

jkorous added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3553 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClass, internal::Matcher, + InnerMatcher) { aaron.ballman wrote: > njames93 wrote: > > jkorous wrote: > >

[PATCH] D81691: [clangd] Set CWD in semaCodeComplete

jkorous added a comment. A slightly tangential thing - we recently got an internal bugreport about clangd handling combination of working directory and relative paths for `-fmodules-cache-path` combination in compile_command.json incorrectly. I tried some quick hacks but failed - it seems that u

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

jkorous added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3553 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClass, internal::Matcher, + InnerMatcher) { aaron.ballman wrote: > jkorous wrote: > > Nit: while "[base sp

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

jkorous added inline comments. Comment at: clang/include/clang/ASTMatchers/ASTMatchers.h:3621 +/// \endcode +AST_MATCHER_P(CXXBaseSpecifier, hasClassOrClassTemplate, + internal::Matcher, InnerMatcher) { I think we should just use `eachOf` matcher for

[PATCH] D81552: [ASTMatchers] Added hasDirectBase and hasClass Matchers

jkorous added a comment. @njames93 `hasDirectBase` seems like a useful matcher to me! OTOH I am not totally convinced about `hasType` -> `hasClass`. Anyway, don't you want to land `hasDirectBase` as a separate patch first and then discuss the rest? One more thing - I'm just thinking if there is

[PATCH] D81017: [Analyzer][WebKit] Check record definition is available in NoUncountedMembers checker

This revision was automatically updated to reflect the committed changes. Closed by commit rGd61ad660503d: [Analyzer][WebKit] Check record definition is available in NoUncountedMembers… (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG

[PATCH] D77644: [clangd] Handle additional includes while parsing ASTs

jkorous added a comment. Hi @kadircet! I am investigating a failure of `PatchesAdditionalIncludes` test that we got internally. It's a failed assert in `ReplayPreamble::replay`. Our clangd source code is for all practical purposes identical to upstream version but not so with clang source. Speci

[PATCH] D79063: [ASTMatchers] Matchers related to C++ inheritance

This revision was automatically updated to reflect the committed changes. Closed by commit rG1a5c97f3a4b8: [ASTMatchers] Matchers related to C++ inheritance (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llv

[PATCH] D77178: [Analyzer][WebKit] NoUncountedMembersChecker

This revision was automatically updated to reflect the committed changes. Closed by commit rG660cda572d6e: [Analyzer][WebKit] NoUncountedMembersChecker (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHANGES SINC

[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

jkorous added inline comments. Comment at: clang/lib/Frontend/SerializedDiagnosticReader.cpp:323 + // A documentation URL has an ID and path size. + if (Record.size() != 2) +return SDError::MalformedDiagnosticRecord; owenv wrote: > owenv wrote:

[PATCH] D77177: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker & shared utils

jkorous added a comment. In D77177#2049805 , @thakis wrote: > This breaks the build everywhere (e.g. > http://45.33.8.238/linux/18283/step_4.txt) and has been in for over an hour. > Please watch bots after landing. (I think changes that are created more

[PATCH] D77177: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker & shared utils

This revision was automatically updated to reflect the committed changes. Closed by commit rGf7c7e8a523f5: [Analyzer][WebKit] RefCntblBaseVirtualDtorChecker (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://reviews.llv

[PATCH] D80126: Add documentation URL records to the .dia format and expose them via libclang

jkorous added a comment. Hi Owen, Do you plan to land the functionality for emitting documentation URLs in clang too? Comment at: clang/lib/Frontend/SerializedDiagnosticReader.cpp:323 + // A documentation URL has an ID and path size. + if (Record.size() != 2) +

[PATCH] D80279: [libclang] Extend clang_Cursor_Evaluate().

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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG05eedf1f5b44: [clang][VerifyDiagnosticConsumer] Support filename wildcards (authored by arames, committed by jkorous). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.ll

[PATCH] D79834: Speed up preamble building by replacing the slow translateFile call by a new, faster isMainFile check

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

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

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

[PATCH] D79834: Speed up preamble building by replacing the slow translateFile call by a new, faster isMainFile check

jkorous added a comment. Could you please add a test for this method to `clang/unittests/Basic/SourceManagerTest.cpp`? Comment at: clang/include/clang/Basic/SourceManager.h:816 + /// Returns true when the given FileEntry corresponds to the main file. + bool isMainFile(File

[PATCH] D79834: Speed up preamble building by replacing the slow translateFile call by a new, faster isMainFile check

jkorous added a comment. IIUC the issue is that `SourceManager::translateFile()` basically consists of two blocks of code: // First, check the main file ID, since it is common to look for a // location in the main file. if (MainFileID.isValid()) { bool Invalid = false; const SLocEn

[PATCH] D79451: [libclang] Remove duplicate dependency on LLVMSupport

This revision was automatically updated to reflect the committed changes. Closed by commit rG02b303321d3f: [libclang] Remove duplicate dependency on LLVMSupport (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo CHA

[PATCH] D78760: Check a class has a definition before iterating over its base classes

jkorous added a comment. In D78760#2003118 , @rsmith wrote: > If so, we'll need to make sure that all code that iterates over base classes > checks for this condition (I bet there are more cases than the two that you > found). Just a potentially relate

[PATCH] D77612: [ASTMatchers] Fix isDerivedFrom for recursive templates

This revision was automatically updated to reflect the committed changes. Closed by commit rG14d89bfbe0b4: [ASTMatchers] Fix isDerivedFrom for recursive templates (authored by jkorous). Herald added a project: clang. Herald added a subscriber: cfe-commits. Changed prior to commit: https://revie

[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] 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] 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] 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] 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] 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

  1   2   3   4   5   >