[PATCH] D153476: [dataflow] document & test determinism of formula structure

sammccall created this revision. Herald added subscribers: martong, mgrang, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This doesn't depend on object

[PATCH] D153469: [dataflow] Replace most BoolValue subclasses with references to Formula (and AtomicBoolValue => Atom and BoolValue => Formula where appropriate)

sammccall created this revision. Herald added subscribers: martong, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This properly frees the Value hierarchy

[PATCH] D153409: BEGIN_PUBLIC [clang][dataflow] Treat fields of anonymous records as being part of their parent.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:5433 +// that `i` is modeled. +S->getChild(*IDecl); + });

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

sammccall updated this revision to Diff 533029. sammccall added a comment. clean up Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153366/new/ https://reviews.llvm.org/D153366 Files: clang/include/clang/Analysis/FlowSensitive/Arena.h

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

sammccall updated this revision to Diff 533028. sammccall added a comment. remove dead comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153366/new/ https://reviews.llvm.org/D153366 Files: clang/include/clang/Analysis/FlowSensitive/Arena.h

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

sammccall updated this revision to Diff 533027. sammccall edited the summary of this revision. sammccall added a comment. more tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153366/new/ https://reviews.llvm.org/D153366 Files:

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

sammccall updated this revision to Diff 533025. sammccall edited the summary of this revision. sammccall added a comment. updating description Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153366/new/ https://reviews.llvm.org/D153366 Files:

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

sammccall updated this revision to Diff 533022. sammccall added a comment. update tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153366/new/ https://reviews.llvm.org/D153366 Files: clang/include/clang/Analysis/FlowSensitive/Arena.h

[PATCH] D153366: [dataflow] Add dedicated representation of boolean formulas

sammccall created this revision. Herald added subscribers: martong, mgrang, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is the first step in

[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/unittests/FindTargetTests.cpp:648 + {"bool operator==(const Foo &) const noexcept = default", +

[PATCH] D153331: [clangd][c++20]Consider rewritten binary operators in TargetFinder

sammccall added a comment. Thanks, this looks pretty good! > I'm not sure if I the hover test which is added in this patch is the right > one, > but at least is passed with this patch and fails without it :) This is nice to have, but you add a unittest to FindTargetTest too? That's the most

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

sammccall added a comment. (Sorry, hit send too soon) I suspect the answer for header modules is that we can study this patch and understand what the equivalents of graph nodes/deps/names/scanning look like for explicit header modules, and understand that we'll be able to abstractify some

[PATCH] D153114: [clangd] [C++20] [Modules] Support C++20 modules for clangd

sammccall added a comment. Thanks for putting this together, I'm going to study it carefully and try it out! That said, there are two large issues that I think should be addressed in the design (though not necessarily *implemented* now). I'll be upfront: these are things without which

[PATCH] D152274: [clang] Don't create import decls without -fmodules

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. The internal state when modules are off but layering check is on is really counterintuitive, and clearly not all configurations have been tested :-( I'm pretty sure this is the right

[PATCH] D151253: [clangd] Move completion signatures and labelDetails

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG3f9f1463719d: [clangd] Move completion signatures and labelDetails (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D151128: [clangd] Show size, offset and padding for bit fields on hover

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG4cb5e436ae71: [clangd] Show size, offset and padding for bit fields on hover (authored by SR_team, committed by sammccall). Repository: rG LLVM

[PATCH] D148918: Check for a ‘buffer’ type instead of ‘buffer-live’.

This revision was automatically updated to reflect the committed changes. Closed by commit rGf69110dcc973: Check for a ‘buffer’ type instead of ‘buffer-live’. (authored by phst, committed by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D151128: [clangd] Show size, offset and padding for bit fields on hover

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Sorry for the delay, I've been out on vacation. Behavior looks great and isn't too complicated :-) Let me know if you have commit access or I should land this for you (in which case:

[PATCH] D151253: [clangd] Move completion signatures and labelDetails

sammccall updated this revision to Diff 525517. sammccall retitled this revision from "[clangd] Move completion signatures and insertion markers into labelDetails" to "[clangd] Move completion signatures and labelDetails". sammccall edited the summary of this revision. sammccall added a comment.

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. > 150ms is more than noticeable Fair enough. I also see we *are* avoiding the quadratic algorithm in other places in inlay hints, and that we can't easily reuse the getHintRange()

[PATCH] D148793: [WIP][clang-tidy] Implement an include-cleaner check.

sammccall added a comment. Drive-by nit: want add this to the `disableUnusableChecks()` blocklist in `clangd/TidyProvider.cpp`? Since inside clangd we want to use the direct feature instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

sammccall added a comment. In D150635#4366730 , @daiyousei-qz wrote: > I'm not sure but 18ms is definitely wrong here, especially for a debug build. > In your log, even semantic highlighting builds in 60ms, which looks > incorrect. I tried again with

[PATCH] D151253: [clangd] Move completion signatures and insertion markers into labelDetails

sammccall added a comment. Hmm, having looked at that screenshot more, the bullets are not rendered properly on some lines... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151253/new/ https://reviews.llvm.org/D151253

[PATCH] D151253: [clangd] Move completion signatures and insertion markers into labelDetails

sammccall added a comment. Looks like this: F27614864: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151253/new/ https://reviews.llvm.org/D151253 ___

[PATCH] D151253: [clangd] Move completion signatures and insertion markers into labelDetails

sammccall created this revision. sammccall added a reviewer: hokein. Herald added subscribers: kadircet, arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D151128: [clangd] Show size, offset and padding for bit fields on hover

sammccall added a comment. I do see the appeal of making this work for bitfields, but I think silently rounding offset/size/padding to whole bytes is pretty misleading. For bitfields we'd really need to talk about layout at a bit level. But we don't want to talk about regular fields in bits (I

[PATCH] D149677: [clang][TypePrinter] Add option to skip over elaborated types

sammccall added a comment. In D149677#4356863 , @aaron.ballman wrote: > I'm not seeing how the changes you've got here interact with > `isScopeVisible()`; However, I also note that the only use of > `isScopeVisible()` in tree is in clangd, so also

[PATCH] D150872: [clang-tidy] Add support for new TODO style to google-readability-todo

sammccall added a comment. I'd be more comfortable reviewing this after it's actually been added to the style guide, it's hard to refer to non-public docs and discussions here. I don't really understand the rationale for the hard split into two modes, vs always accepting both styles and maybe

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

sammccall added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:616 +Position HintStart = sourceLocToPosition(SM, BraceRange.getEnd()); +Position HintEnd = {HintStart.line, +HintStart.character + daiyousei-qz

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

sammccall added a comment. Thanks, I'll try to repro those results. 100ms is significant in absolute terms, but >1s seems unacceptably slow. I believe VSCode always sends ranges along with latency-sensitive hint requests, I think we currently just post-filter. If we propagate the limits deeper

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

sammccall added a comment. > Though I'm proposing a name like "DeclBlockEnd" to make it clearer I prefer the current name. DeclBlockEnd is both too long and contains a cryptic abbreviation (should be DeclarationBlockEnd). And I don't think distinguishing between declaration blocks and

[PATCH] D151166: [clangd] Interactive AST matchers with #pragma clang query

sammccall added a comment. @kadircet this is a slightly silly feature that I put together on vacation I'd like your call on whether this is something we should have. If so, feel free to hand off the review. It's definitely a power-user feature and nigh undiscoverable. I'd plan to write some

[PATCH] D151166: [clangd] Interactive AST matchers with #pragma clang query

sammccall updated this revision to Diff 524519. sammccall edited the summary of this revision. sammccall added a comment. Add demo link Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151166/new/ https://reviews.llvm.org/D151166 Files:

[PATCH] D151166: [clangd] Interactive AST matchers with #pragma clang query

sammccall created this revision. Herald added subscribers: kadircet, arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. The idea: you can type in AST

[PATCH] D150683: [clangd] Tweak "provides" hover card when symbols have the same name

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0e66105a9260: [clangd] Tweak provides hover card when symbols have the same name (authored by sammccall). Repository: rG LLVM Github Monorepo

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

sammccall added a comment. In D150635#4348702 , @daiyousei-qz wrote: > Regarding to **Naming**, I agree that `EndDefinitionComment` sounds so > tedious but I couldn't come up with a better name. However, I think > `EndBlock` also feel a little strange

[PATCH] D150655: [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.

sammccall accepted this revision. sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:138 +static void forwardValue(const Expr , const Expr , Environment ) { + if (auto *Val = Env.getValueStrict(From)) mboehme wrote: >

[PATCH] D150683: [clangd] Tweak "provides" hover card when symbols have the same name

sammccall added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1539 +Front, [&](llvm::StringRef Sym) { P.appendCode(Sym); }, +[&] { P.appendText(", "); }); if (UsedSymbolNames.size() > Front.size()) { sammccall wrote: >

[PATCH] D150683: [clangd] Tweak "provides" hover card when symbols have the same name

sammccall added inline comments. Comment at: clang-tools-extra/clangd/Hover.cpp:1539 +Front, [&](llvm::StringRef Sym) { P.appendCode(Sym); }, +[&] { P.appendText(", "); }); if (UsedSymbolNames.size() > Front.size()) { VitaNuo wrote: > What

[PATCH] D150683: [clangd] Tweak "provides" hover card when symbols have the same name

sammccall created this revision. sammccall added a reviewer: VitaNuo. Herald added subscribers: kadircet, arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra.

[PATCH] D149612: [Sema] avoid merge error type

sammccall added a comment. Unless I'm wildly misremembering (@hokein knows better), the type should also be dependent in C. The intuition is that the code has some meaning that depends on how the error is resolved (by the programmer changing the code!), and that "dependent" is a good

[PATCH] D150657: [clang][dataflow] Use `Strict` accessors in SignAnalysisTest.cpp.

sammccall added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp:240 +// Returns the `Value` associated with `E` (which may be either a prvalue or +// glvalue). Creates a `Value` or `StorageLocation` as needed if `E` does not

[PATCH] D150655: [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.

sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:138 +static void forwardValue(const Expr , const Expr , Environment ) { + if (auto *Val = Env.getValueStrict(From)) mboehme wrote: > ymandel wrote: > > mboehme wrote: >

[PATCH] D149912: [clangd] downgrade missing-includes diagnostic to Information level

This revision was automatically updated to reflect the committed changes. Closed by commit rG8f8479789a08: [clangd] downgrade missing-includes diagnostic to Information level (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D150653: [clang][dataflow] Add `Strict` versions of `Value` and `StorageLocation` accessors.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:644 +StorageLocation *Environment::getStorageLocationStrict(const Expr ) const { +

[PATCH] D150655: [clang][dataflow] Use `Strict` accessors in more places in Transfer.cpp.

sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:138 +static void forwardValue(const Expr , const Expr , Environment ) { + if (auto *Val = Env.getValueStrict(From)) the name "forward" isn't clear to me - if anything

[PATCH] D150657: [clang][dataflow] Use `Strict` accessors in SignAnalysisTest.cpp.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/SignAnalysisTest.cpp:240 +// Returns the `Value` associated with `E` (which may be either a prvalue or +//

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

sammccall added inline comments. Comment at: clang-tools-extra/clangd/InlayHints.cpp:292 + + bool VisitRecordDecl(RecordDecl *D) { +if (Cfg.InlayHints.EndDefinitionComments && Not sure why if need to handle this + EnumDecl, can we just handle VisitTagDecl

[PATCH] D150635: [clangd] Implement end-definition-comment inlay hints

sammccall added a comment. Thanks for doing this! Some high-level comments here, will review implementation next: **There seems to be a lot of "decoration" on the hint text.** If declaring a class "Foo" then we add ` /* class ` before and ` */ ` after Foo, for a total of 14 characters apart

[PATCH] D150257: [clangd] Initialize clang-tidy modules only once

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/ParsedAST.cpp:478 trace::Span Tracer("ClangTidyInit"); -tidy::ClangTidyCheckFactories CTFactories; -for (const

[PATCH] D149912: [clangd] downgrade missing-includes diagnostic to Information level

sammccall added a comment. This change is just one option to be evaluated for making these diagnostics more user-friendly. Mostly sending the patch now to attach some screenshots... Effect in VSCode (blue underline, visible in problems view, filterable) F27332814: image.png

[PATCH] D149912: [clangd] downgrade missing-includes diagnostic to Information level

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added a subscriber: arphaman. Herald added a project: All. sammccall requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. In

[PATCH] D149650: Give NullabilityKind a printing operator<

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0a532207b869: Give NullabilityKind a printing operator (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D149650: Give NullabilityKind a printing operator<

sammccall added a comment. Thanks Aaron! The check is not going to be ready for upstreaming really soon - plenty of thorny issues to work out first, and checking in a half-working version would do more harm than good. But that's definitely the goal once we have some deployment experience.

[PATCH] D149650: Give NullabilityKind a printing operator<

sammccall added a comment. In D149650#4316138 , @aaron.ballman wrote: > In D149650#4315211 , @sammccall > wrote: > >> In D149650#4312389 , >> @aaron.ballman wrote: >>

[PATCH] D149744: [clang][dataflow][NFC] Eliminate unnecessary helper `stripReference()`.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. This looks equivalent to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149744/new/ https://reviews.llvm.org/D149744

[PATCH] D149650: Give NullabilityKind a printing operator<

sammccall updated this revision to Diff 519046. sammccall added a comment. Make diagnostic-printing use the (other) existing function, come out neutral! (This adds quotes to some diagnostic, but their omission seems to be an error) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D149650: Give NullabilityKind a printing operator<

sammccall added a reviewer: aaron.ballman. sammccall added a comment. In D149650#4312389 , @aaron.ballman wrote: > I guess I'm not seeing much motivation for this change. We already have > `clang::getNullabilitySpelling()` and `const

[PATCH] D149650: Give NullabilityKind a printing operator<

sammccall added inline comments. Comment at: clang/lib/Basic/IdentifierTable.cpp:852 +llvm::raw_ostream <<(llvm::raw_ostream , NullabilityKind NK) { + switch (NK) { IdentifierTable.cpp is a weird place to implement this function (I put it next to

[PATCH] D149650: Give NullabilityKind a printing operator<

sammccall created this revision. sammccall added a reviewer: mboehme. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. This is more useful for debug/test than getNullabilitySpelling: - default form

[PATCH] D148457: [clangd] Support macro evaluation on hover

sammccall added a comment. > Obviously we're expecting evaluation on such macro (which is just what the > original issue addresses). That wasn't (and isn't) obvious to me - the issue didn't mention the macro in question, i assumed it was `#define alignof(x) __alignof(x)` or similar. Looks

[PATCH] D148457: [clangd] Support macro evaluation on hover

sammccall added a comment. (This looks really cool, nice work! only a driveby comment from me I'm afraid) In D148457#4304915 , @zyounan wrote: > The dot prior to AST node kind indicates partial selection and the asterisk > indicates complete selection

[PATCH] D148088: [RFC][clangd] Move preamble index task to a seperate task

sammccall added inline comments. Comment at: clang-tools-extra/clangd/ClangdServer.cpp:88 +if (PreambleIndexTask) + PreambleIndexTask->runAsync("task:" + Path + Version, + std::move(Task)); ilya-biryukov wrote: > This

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGb56b15ed719b: [dataflow] HTMLLogger - show the value of the current expr (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:96 +for (const auto& Prop : V.properties()) + JOS.attributeObject(Prop.first(), [&] { dump(*Prop.second); }); + mboehme wrote: > sammccall wrote: > > mboehme

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

sammccall updated this revision to Diff 516689. sammccall added a comment. display properties at the end of the kv list Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148949/new/ https://reviews.llvm.org/D148949 Files:

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

sammccall updated this revision to Diff 516395. sammccall added a comment. use p: and f: prefixes for properties/fields respectively (we can remove these again if too annoying) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148949/new/

[PATCH] D148951: [dataflow] HTMLLogger: fix off-by-one in the BB listing

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG271a73dae32f: [dataflow] HTMLLogger: fix off-by-one in the BB listing (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

sammccall updated this revision to Diff 516392. sammccall marked an inline comment as done. sammccall added a comment. fix bad cast Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148949/new/ https://reviews.llvm.org/D148949 Files:

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:96 +for (const auto& Prop : V.properties()) + JOS.attributeObject(Prop.first(), [&] { dump(*Prop.second); }); +

[PATCH] D148951: [dataflow] HTMLLogger: fix off-by-one in the BB listing

sammccall added a comment. Before: F27217511: image.png After: F27217508: image.png Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148951/new/

[PATCH] D148951: [dataflow] HTMLLogger: fix off-by-one in the BB listing

sammccall created this revision. sammccall added a reviewer: mboehme. Herald added subscribers: martong, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The

[PATCH] D148949: [dataflow] HTMLLogger - show the value of the current expr

sammccall created this revision. Herald added subscribers: martong, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository: rG LLVM Github Monorepo

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall added a comment. Sorry about the delay. I hope this is fixed by 28114722baabb468732a9cc24784abafd6c47792 . I wasn't able to reproduce the problem locally, so I'm not sure. add_llvm_library says DEPENDS is the same

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall added inline comments. Comment at: clang/utils/bundle_resources.py:6 +# Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +# See https://llvm.org/LICENSE.txt for license information. +# SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall added a comment. In D146591#4278074 , @xazax.hun wrote: > I am a bit overloaded at the moment, feel free to commit. I can still add > comments later that could be addressed in a follow up. Thanks! Happy to address as they come up.

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGa443b3d18ef4: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state (authored by sammccall). Changed prior to commit:

[PATCH] D148554: [dataflow] Extract arena for Value/StorageLocation out of DataflowAnalysisContext

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked 2 inline comments as done. Closed by commit rGbf47c1ed8556: [dataflow] Extract arena for Value/StorageLocation out of… (authored by sammccall).

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall added a comment. @xazax.hun do you still want to look at this again? I'm itching a little to get some use out of it :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591

[PATCH] D148554: [dataflow] Extract arena for Value/StorageLocation out of DataflowAnalysisContext

sammccall added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Arena.h:15-16 + +// The Arena owns the objects that model data within an analysis. +// Currently this is mostly Values and StorageLocations. +class Arena { mboehme wrote: >

[PATCH] D148554: [dataflow] Extract arena for Value/StorageLocation out of DataflowAnalysisContext

sammccall updated this revision to Diff 514602. sammccall added a comment. Tweak comment Drop flow-condition substitution instead of moving into Arena, it's dead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148554/new/

[PATCH] D148554: [dataflow] Extract arena for Value/StorageLocation out of DataflowAnalysisContext

sammccall created this revision. sammccall added reviewers: ymandel, gribozavr2, xazax.hun. Herald added subscribers: martong, rnkovacs. Herald added a reviewer: NoQ. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall updated this revision to Diff 513544. sammccall added a comment. oops, after testing this time Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall updated this revision to Diff 513543. sammccall added a comment. Extract reinflate() function Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

[PATCH] D148158: [include-cleaner] Handle incomplete template specializations

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:85 // This is the underlying decl used by TemplateSpecializationType, can be // null when

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/Preamble.h:144 + /// any. + static const FileEntry *getPatchEntry(llvm::StringRef MainFilePath, +

[PATCH] D148143: [clangd] Treat preamble patch as main file for include-cleaner analysis

sammccall added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:379 + (H.physical() == MainFile || + H.physical()->getName().endswith(PreamblePatch::HeaderName))) { Satisfied = true; Comparing

[PATCH] D148112: [include-cleaner] Improve handling for templates

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:91 +auto *CTSD = llvm::cast(TD); +if (CTSD->isExplicitInstantiationOrSpecialization()) +

[PATCH] D93829: [clangd] Support outgoing calls in call hierarchy

sammccall added a comment. Herald added a project: All. Time to wake up an old review! Going to put high-level thoughts on https://github.com/clangd/clangd/discussions/1206, but since one of them is memory usage I wanted to have a look at the concrete data structure here. TL;DR: I think we

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:77 + +void escape(char C, llvm::raw_ostream ) { + switch (C) { xazax.hun wrote: > We already sort of have a way to escape HTML here: >

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall updated this revision to Diff 512377. sammccall marked an inline comment as done. sammccall added a comment. use llvm::printHTMLEscaped Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall added a comment. doh, forgot to send Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.cpp:128 +*OS << "" << HTMLLogger_css << "\n"; +*OS << "" << HTMLLogger_js << "\n"; + gribozavr2 wrote: > Now that we have an HTML template file, you

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall updated this revision to Diff 511126. sammccall marked 8 inline comments as done. sammccall added a comment. address comments (sorry about long turnaround!) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall added inline comments. Comment at: clang/lib/Analysis/FlowSensitive/HTMLLogger.html:19 +Function + + gribozavr2 wrote: > Could you add the file name and line number of the start location of the > function? > > It might be helpful not only for the

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall updated this revision to Diff 508518. sammccall added a comment. Only show the filename, not the full path, to avoid overflowing the box Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall updated this revision to Diff 508517. sammccall marked 4 inline comments as done. sammccall edited the summary of this revision. sammccall added a comment. Address Dmitri's comments Update demo link Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146864: [dataflow] Delete legacy aliases

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe884e005f363: [dataflow] Delete legacy aliases (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D146864: [dataflow] Delete legacy aliases

sammccall created this revision. sammccall added a reviewer: ymandel. Herald added subscribers: martong, jeroen.dobbelaere, xazax.hun. Herald added a reviewer: NoQ. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D146625: [dataflow] handle missing case in value debug strings

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd630134f1f1f: [dataflow] handle missing case in value debug strings (authored by sammccall). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D146591: [dataflow] add HTML logger: browse code/cfg/analysis timeline/state

sammccall updated this revision to Diff 508273. sammccall added a comment. revert accidental edit Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D146591/new/ https://reviews.llvm.org/D146591 Files:

<    1   2   3   4   5   6   7   8   9   10   >