[PATCH] D155540: [clangd] Remove extra dependancies for clangd

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. I don't have a strong opinion here (or solid understanding of dynamic linking!) I suppose the idea is that clangdMain has dependencies from the source files in this directory, and the

[PATCH] D133843: [clangd] Prefer definitions for gototype and implementation

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Yeah, having used this feature for a while, you're right about all of this of course, the current behavior is infuriating. Sorry! Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D155898: [clangd] Fix go-to-type target location

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, wangpc, MaskRay, ilya-biryukov. Herald added a project: clang-tools-extra. Previously it'd ~always jump

[PATCH] D155885: [include-cleaner] Switch Include from FileEntry* -> FileEntryRef

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added subscribers: PiotrZSL, carlosgalvezp, arphaman. Herald added a reviewer: njames93. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang-tools-extra. Herald added a

[PATCH] D155878: [clangd] Loose include-cleaner matching for verbatim headers

sammccall updated this revision to Diff 542653. sammccall added a comment. Herald added a subscriber: ChuanqiXu. extract isPreferredProvider helper Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155878/new/ https://reviews.llvm.org/D155878 Files:

[PATCH] D155878: [clangd] Loose include-cleaner matching for verbatim headers

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. This updates clangd to take advantage

[PATCH] D155819: [include-cleaner] Loose matching for verbatim headers

sammccall updated this revision to Diff 542444. sammccall added a comment. Herald added a subscriber: ormris. clean up some leftovers Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155819/new/ https://reviews.llvm.org/D155819 Files:

[PATCH] D155819: [include-cleaner] Loose matching for verbatim headers

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. A verbatim header usually corresponds to a symbol from a header with

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

sammccall added a comment. Example of using conusmeASTs + executeFromCommandLine (It's not that many extra lines of code, but they're not really easy ones to write, and to work out whether they're minimal/correct) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155385: [clangd] enable unused-include warnings for standard library headers

This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rGe289ee99cec4: [clangd] enable unused-include warnings for standard library headers (authored by sammccall). Changed prior to commit:

[PATCH] D155385: [clangd] enable unused-include warnings for standard library headers

sammccall marked an inline comment as done. sammccall added inline comments. Comment at: clang-tools-extra/clangd/IncludeCleaner.cpp:75-77 +if (tooling::stdlib::Header::named(Inc.Written)) return true; return false; kadircet wrote: > believe it

[PATCH] D155671: [include-cleaner] allow spelling strategies to customize verbatim/system headers

sammccall created this revision. sammccall added a reviewer: kadircet. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang-tools-extra. Herald added a subscriber: cfe-commits. Our use case is wanting to apply a spelling strategy to rewrite the

[PATCH] D140745: Generate Config {Fragment structure, json schema, docs, YAML parser} from schema spec

sammccall added a comment. In D140745#4508559 , @nridge wrote: > In D140745#4505829 , @sammccall > wrote: > >> ping on this one for when you have time > > (Just wanted to double-check, is this ping directed to

[PATCH] D140745: Generate Config {Fragment structure, json schema, docs, YAML parser} from schema spec

sammccall added a comment. ping on this one for when you have time I haven't rebased against latest config yet, it's a bit of a moving target :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D140745/new/ https://reviews.llvm.org/D140745

[PATCH] D78038: [clangd] WIP: fix several bugs relating to include insertion

sammccall added a comment. OK, time to revive this patch, sorry for letting it die. Meanwhile, the lexer change has landed separately, so that's gone. I've removed the gratuitous clangd indexing changes in order to focus this on the serialization hacks. Comment at:

[PATCH] D78038: [clangd] WIP: fix several bugs relating to include insertion

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

[PATCH] D155455: [Serialization] Read main file's HeaderFileInfo from PCH even if changed

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, llvm-commits, ilya-biryukov. Herald added projects: clang, LLVM, clang-tools-extra. Because file size is

[PATCH] D78038: [clangd] WIP: fix several bugs relating to include insertion

sammccall updated this revision to Diff 540981. sammccall added a comment. rebase and narrow scope Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D78038/new/ https://reviews.llvm.org/D78038 Files:

[PATCH] D155425: [include-cleaner] Record concept uses

sammccall created this revision. sammccall added reviewers: kadircet, massberg. Herald added a project: All. sammccall requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscriber: cfe-commits. Most cases handled by adding VisitConceptReference to

[PATCH] D155421: [clangd] Add BlockEnd comments for control flow statements

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] D154903: clangd: Provide the resource dir via environment variable

sammccall added a comment. I'm a little nervous about this. On the one hand it's simple and I want to get you unblocked but: - the more ways to initialize resource dir and the more places this needs to be propagated to, the more likely this is to get out of sync - we might want to remove the

[PATCH] D155370: [CodeComplete] Don't emit parameters when not FunctionCanBeCall

sammccall added a comment. I'm not sure switching to RK_Pattern is ideal, but I think there are a couple of (easy?) alternatives that achieve the same. > Regard to the heuristic itself, it's currently a bit inconvenient that I have > to switch headers and sources back and forth and do

[PATCH] D155392: [clangd] add a config knob to disable modules

sammccall added inline comments. Comment at: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp:497 TEST(DocumentSymbols, ExportContext) { + Config EnableModules; + EnableModules.CompileFlags.NaiveModules = true; For existing tests of how modules

[PATCH] D155392: [clangd] add a config knob to disable modules

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. We don't

[PATCH] D155385: [clangd] enable unused-include warnings for standard library headers

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

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

This revision was automatically updated to reflect the committed changes. Closed by commit rG9e6a342fdac8: [clangd] Implement end-definition-comment inlay hints (authored by daiyousei-qz, committed by sammccall). Changed prior to commit: https://reviews.llvm.org/D150635?vs=525922=540750#toc

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

sammccall added a comment. Oh no, I just saw this never landed - I suspect you were waiting for me to commit it, and I was expecting you to do it! I've rebased and I'll commit it for you now. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

sammccall added a comment. In D153946#4503585 , @sammccall wrote: > - maybe we can come up with some heuristics to improve the "no reserved > names" rule, with or without configuration. (Maybe apply it to only -isystem > headers? Linux uses

[PATCH] D155381: [clangd] Allow indexing of __reserved_names outside system headers

sammccall created this revision. sammccall added a reviewer: nridge. 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] D153946: [clangd] Add a flag to allow indexing of reserved identifiers

sammccall added a comment. Thanks for putting this together, it definitely seems like something people want. My main concern is that the configuration has the wrong scope. We're checking whether reserved-ident indexing is on for the **TU** we're indexing, but really we want to specify which

[PATCH] D154853: [clangd][c++20]Consider the constraint of a constrained auto in FindTarget.

sammccall added a comment. Sorry about the delay here. Making the whole AutoTypeLoc resolve to the concept doesn't seem right - the `auto` part does not refer to the concept, and in principle refers to another type entirely. Really I think there should be a child node representing just the

[PATCH] D153485: [dataflow] use true/false literals in formulas, rather than variables

sammccall reopened this revision. sammccall added a comment. This revision is now accepted and ready to land. (this was reverted, I want to reland sometime, but there's a bad interaction with a bug in nullability analysis) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

sammccall updated this revision to Diff 540660. sammccall added a comment. format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155361/new/ https://reviews.llvm.org/D155361 Files: clang/include/clang/Tooling/AllTUsExecution.h

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

sammccall updated this revision to Diff 540659. sammccall added a comment. squash commits Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155361/new/ https://reviews.llvm.org/D155361 Files: clang/include/clang/Tooling/AllTUsExecution.h

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

sammccall updated this revision to Diff 540657. sammccall added a comment. tweak comment, remove unused overload Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D155361/new/ https://reviews.llvm.org/D155361 Files:

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

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

[PATCH] D155361: [Tooling] Avoid boilerplate in common cases

sammccall created this revision. Herald added a project: All. sammccall requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The tooling APIs have a lot of extension points for customization: e.g. Executor, FrontendActionFactory,

[PATCH] D153058: [clang][CFG] Support construction of a weak topological ordering of the CFG.

sammccall added a comment. Having (mostly!) understood what this is doing, the algorithm looks like it's doing the right thing. High-level ideas would be: - there are three different representations/sets of APIs exposed: intervals, ordering, and compare. It seems like we only have immediate

[PATCH] D155164: [clangd][NFC] Remove dead code

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

[PATCH] D155152: [dataflow] Remove deprecated BoolValue flow condition accessors

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6272226b9f94: [dataflow] Remove deprecated BoolValue flow condition accessors (authored by sammccall). Repository: rG LLVM Github Monorepo

[PATCH] D155152: [dataflow] Remove deprecated BoolValue flow condition accessors

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

[PATCH] D154948: [dataflow] improve determinism of generated SAT system

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG7d935d083659: [dataflow] improve determinism of generated SAT system (authored by sammccall). Changed prior to commit:

[PATCH] D154969: [dataflow] document flow condition

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfa689726768b: [dataflow] document flow condition (authored by sammccall). Changed prior to commit:

[PATCH] D154969: [dataflow] document flow condition

sammccall created this revision. sammccall added a reviewer: ymandel. 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.

[PATCH] D154948: [dataflow] improve determinism of generated SAT system

sammccall created this revision. sammccall added reviewers: ymandel, xazax.hun. Herald added subscribers: ChuanqiXu, martong, mgrang. 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] D154833: [clang][dataflow] Add `AnalysisInputs::withSolverFactory()`.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/unittests/Analysis/FlowSensitive/TestingSupport.h:180 + /// SAT solver factory. + std::function()> SolverFactory = [] { +return

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

sammccall added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:532 + void addToFlowCondition(const Formula &); + LLVM_DEPRECATED("Use Formula version instead", "") void addToFlowCondition(BoolValue ); uabelho

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

sammccall added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:532 + void addToFlowCondition(const Formula &); + LLVM_DEPRECATED("Use Formula version instead", "") void addToFlowCondition(BoolValue ); uabelho

[PATCH] D154580: [clangd][c++20]Add missing semantic highlighing for concepts.

sammccall accepted this revision. sammccall added a comment. This revision is now accepted and ready to land. Nice! I forgot about the TypeLoc hierarchy, getNameLoc() isn't so obvious... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D154580/new/

[PATCH] D154580: [clangd][c++20]Add missing semantic highlighing for concepts.

sammccall added inline comments. Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:966 + +if (auto *TSI = D->getTypeSourceInfo()) { + auto ATL = TSI->getTypeLoc().getContainedAutoTypeLoc(); if we add this logic it definitely needs a comment

[PATCH] D153485: [dataflow] use true/false literals in formulas, rather than variables

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG71579569f439: [dataflow] use true/false literals in formulas, rather than variables (authored by sammccall). Changed prior to commit:

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rG5e4ad816bf10: [dataflow] Replace most BoolValue subclasses with references to Formula (and… (authored by

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

sammccall marked 3 inline comments as done. sammccall added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/Arena.h:61 + /// Passing in the same formula will result in the same BoolValue. + /// FIXME: This canonicalization isn't sound e.g. if we create

[PATCH] D154450: [clangd][c++20] Drop first template argument in code completion in some contexts.

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang-tools-extra/clangd/CodeComplete.cpp:484 + S.Signature = RemoveFirstTemplateArg(S.Signature); + S.SnippetSuffix =

[PATCH] D154450: [clangd][c++20] Drop first template argument in code completion in some contexts.

sammccall added a comment. Cool! This makes sense, and is much cheaper than actually working out how to render the abbreviated arg list and store it in the index. Nice that TopLevel gives us some of these contexts, but I suspect it's not all. (If not, it's fine to handle just this case for now

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

sammccall added a comment. In D153366#4471320 , @TWeaver wrote: > My apologies but I've had to revert this change for now until the author can > address the buildbot failures. Thanks for the revert, and sorry for the disruption - I expected to be

[PATCH] D154443: [clangd] Downgrade deprecated warnings to hints

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

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

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. sammccall marked an inline comment as done. Closed by commit rG2fd614efc1bb: [dataflow] Add dedicated representation of boolean formulas (authored by sammccall).

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

sammccall marked 11 inline comments as done. sammccall added inline comments. Herald added a subscriber: wangpc. Comment at: clang/include/clang/Analysis/FlowSensitive/Formula.h:69 + + Atom atom() const { +assert(kind() == AtomRef); gribozavr2 wrote: >

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall closed this revision. sammccall added a comment. Herald added a subscriber: wangpc. This relanded as 1e010c5c4fae43c52d6f5f1c8e8920c26bcc6cc7 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D154335: [clang][tooling] Fix early termination when there are nested expansions

sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:116 - -// Now: how do we adjust the previous/next bounds? Three cases: -// A) If they are also part of the

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

sammccall added a comment. In D153805#4464487 , @bazuzi wrote: > It looks like D153485 changes the context > for the last few comments significantly. What's the appetite for adding yet > another child commit to the

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

sammccall added a comment. LG as is, sorry for the noise! Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:183 + /// been stored in flow conditions. + Solver::Result querySolver(llvm::DenseSet Constraints); + gribozavr2

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

sammccall added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h:183 + /// been stored in flow conditions. + Solver::Result querySolver(llvm::DenseSet Constraints); + sammccall wrote: > sammccall wrote: > >

[PATCH] D153805: Expose DataflowAnalysisContext.querySolver().

sammccall accepted this revision. sammccall added a comment. Thanks! As discussed offline, I had some concerns about whether there were any cases where it was safe to use formulas separate from the FC that might constrain them. But we found some: these are formulas produced by the downstream

[PATCH] D153674: [dataflow] Disallow implicit copy of Environment, use fork() instead

sammccall added a comment. Sorry about the delay, and thanks for the `.i`. It took me a while to work out what was going on, but it looks like it's just a bug where GCC forgets to automatic-move. Fixed in 2f7d30dee8262746c3e8ee1f6f25be8c1ace9990

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

sammccall updated this revision to Diff 535491. sammccall added a comment. rebase, primarily on the SAT-inputs-are-ordered change (SetVector etc) clarify that Formula::print output is supposed to be reliably stable. This is useful for testing downstream analyses: having a representation of

[PATCH] D153960: [clang][dataflow] Implement support for pointers to members.

sammccall accepted this revision. sammccall added a comment. Per offline discussion, I think modelling pointer-to-member as PointerValue does make sense, though that's not completely obvious. Can we document this on PointerValue? (And that it points to a StorageLocation for the decl, which has

[PATCH] D153908: [dataflow] Use consistent, symmetrical, non-mutating erased signature for join()

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6f22de67c585: [dataflow] Use consistent, symmetrical, non-mutating erased signature for join() (authored by sammccall). Repository: rG LLVM

[PATCH] D153908: [dataflow] Use consistent, symmetrical, non-mutating erased signature for join()

sammccall created this revision. sammccall added reviewers: ymandel, xazax.hun. Herald added a subscriber: martong. 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.

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall added a comment. Thanks! I'll land this to eliminate the variables, and follow up with trying to simplify join(). In D153493#4450440 , @ymandel wrote: > In D153493#4450358 , @sammccall > wrote: > >>>

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall added a comment. In D153493#4450191 , @ymandel wrote: > I don't want to block the progress you're making elsewhere and I think the > concerns here are sufficiently localized to revisit another time. So, feel > free to reify any

[PATCH] D153584: [dataflow] Make SAT solver deterministic

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGd85c233e4b05: [dataflow] Make SAT solver deterministic (authored by sammccall). Changed prior to commit:

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall updated this revision to Diff 534691. sammccall added a comment. address comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153493/new/ https://reviews.llvm.org/D153493 Files:

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall marked an inline comment as done. sammccall added a comment. > Regarding the design. This looks like an optimal solution in terms of copying > but introduces lifetime risks and complexity that I'm uncomfortable with. FWIW I agree with the complexity (not the lifetime risks). I think

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall added a comment. Sorry, going to **revert**, review would still be appreciated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153493/new/ https://reviews.llvm.org/D153493 ___ cfe-commits

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall added a comment. Sorry, I got confused between branches and landed this one without approval Going to review, review would still be appreciated Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153493/new/ https://reviews.llvm.org/D153493

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGae54f01dd8c5: [dataflow] avoid more accidental copies of

[PATCH] D153674: [dataflow] Disallow implicit copy of Environment, use fork() instead

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc2bb68078eb9: [dataflow] Disallow implicit copy of Environment, use fork() instead (authored by sammccall). Changed prior to commit:

[PATCH] D153674: [dataflow] Disallow implicit copy of Environment, use fork() instead

sammccall added inline comments. Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowAnalysis.h:211 PostVisitCFG(Element, DataflowAnalysisState{ -*Lattice, State.Env}); +*Lattice,

[PATCH] D153674: [dataflow] Disallow implicit copy of Environment, use fork() instead

sammccall created this revision. sammccall added reviewers: xazax.hun, ymandel. Herald added a subscriber: martong. 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.

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

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

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

sammccall added a comment. BTW, since this is a substantial API/concept change, I'll wait on approval from all of @mboehme, @xazax.hun, @gribozavr2 - if you don't plan to review just LMK. (Not in a hurry, just wanted to mention so we don't deadlock :-) Comment at:

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

sammccall updated this revision to Diff 534117. sammccall marked 10 inline comments as done. sammccall added a comment. address Martin's review comments Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153366/new/ https://reviews.llvm.org/D153366

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

sammccall added inline comments. Comment at: clang/unittests/Analysis/FlowSensitive/ArenaTest.cpp:169 + auto = A.makeOr(BY, BX); + auto = A.makeEquals(BOr, BAnd); + EXPECT_EQ(Expected, llvm::to_string(B.getFormula(BEqual))); gribozavr2 wrote: > gribozavr2

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

sammccall updated this revision to Diff 534107. sammccall marked 3 inline comments as done. sammccall added a comment. fix busted test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153476/new/ https://reviews.llvm.org/D153476 Files:

[PATCH] D153491: [dataflow] Avoid copying environment

sammccall added a comment. In D153491#4445051 , @ymandel wrote: > In D153491#4443704 , @xazax.hun > wrote: > >> This sounds extremely error-prone to me. In case copying the analysis state >> has side effects

[PATCH] D153584: [dataflow] Make SAT solver deterministic

sammccall added a comment. In D153584#4443796 , @xazax.hun wrote: > Is there a measurable perf cost for this determinism? I'm going to say no... All the extra cost is paid in addTransitiveFlowConditionConstraints. (We were building a set, now we're

[PATCH] D153584: [dataflow] Make SAT solver deterministic

sammccall created this revision. sammccall added reviewers: ymandel, xazax.hun. Herald added subscribers: martong, mgrang. 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.

[PATCH] D153491: [dataflow] Avoid copying environment

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

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

sammccall added a comment. > Would you like me to review https://reviews.llvm.org/D153469 or should we > wait for this patch to converge first? Similarly it'd be great to get high-level feedback there: whether the design of FormulaBoolValue is right, whether the changes to use Atom/Formula

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall added a comment. BTW i thought this was just nice for debugging and saving copying a few maps, but Dmitri says that the way even trivial indirections like `(FC1 = FC2)`, `(FC2 = ...)` are expressed in the SAT solver means they can add significant cost there too... Repository: rG

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

sammccall added reviewers: mboehme, xazax.hun, gribozavr2. sammccall added a comment. Here we start to see benefits: Value becomes less reliant on inheritance, flow conditions are no longer Values that can uselessly bear properties, atomic variables are numbered in order of creation, we'll soon

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

sammccall added reviewers: gribozavr2, xazax.hun, mboehme. sammccall added a comment. Splitting out Formula from BoolValue is a pretty big change... I've tried to keep this patch a manageable size by really just using Formula for interacting with the SAT solver and some printing, but this means

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall added reviewers: ymandel, xazax.hun. sammccall added a comment. The idea with the change to Environment::join() is that the current signature forces the base environment to be mutable, but this sometimes forces the caller to make a copy, and the implementation actually copies

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

sammccall updated this revision to Diff 533463. sammccall added a comment. refactor, restore comment Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D153493/new/ https://reviews.llvm.org/D153493 Files:

[PATCH] D153493: [dataflow] avoid more accidental copies of Environment

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 is clunky but greatly improves

[PATCH] D153491: [dataflow] Avoid copying environment

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 appears to be just an accidental copy

[PATCH] D153488: [dataflow] HTMLLogger: meaningful names for flow condition variables

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. Instead of meaningless "V7", use the name

[PATCH] D153485: [dataflow] use true/false literals in formulas, rather than variables

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. And simplify formulas containing true/false

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