Only show the filename, not the full path, to avoid overflowing the box

Address Dmitri's comments
Update demo link

revert accidental edit

Move data into JSON object, render into DOM with templates.

Going to have a go at passing data via JSON

Address all comments except moving data from HTML => JSON

address review comments

In D146591#4213319, @xazax.hun wrote:

While I am OK with this approach, I wonder if it would be too much work to split the HTML generation and emitting data up. E.g., the logger could emit YAML or JSON that could be parsed by a python script to create the HTML (or the HTML itself could use JS to parse it and generate the DOM).

Use a process-shared counter for HTML output filenames to avoid clobbering.

Added a bunch of reviewers for feedback on how this should work.

In D144730#4211222, @NoQ wrote:

I love such debugging facilities! They're a massive boost to developer productivity compared to ad-hoc debug prints, and they allow new contributors to study how the tool works. I'm excited to see how this thing turns out.

Thanks! I cleaned up a bit and added tests, as well as a -dataflow-log flag to make this easy to access when running unit tests, tidy checks etc.
The HTML experiments seem to have validated that the logging API is sufficient to produce something useful there.

Address review comments
Added -dataflow-log flag to enable logging without code changes
Removed example tool from this patch

In D145843#4207466, @kadircet wrote:

In D145843#4207101, @nridge wrote:

My understanding is that a more elaborate configuration scheme has been proposed in https://github.com/clangd/clangd/issues/1367, and the feedback there was (quoting Sam from this comment):
The approach taken in this patch seemed to me to be in line with this direction of a "simple config-based solution".

I am afraid this approach is a little "too simple". The intent on @sammccall 's comment there is probably around getting rid of the re-writing of include spellings completely, not for specifying quoted or system includes based on the include spelling.
Even if not, I feel like my argument above still applies. I still can't think of many projects benefiting from always using angles/quotes for include spellings.

doh, thanks!

Sorry for the delay, extracting the repro from the build system seems about as much work as minimizing it :-)

We're seeing new clang crashes that bisect to this commit, with modules only.

If someone wants to work on producing a more modular, component-based design, that's probably a good idea (but tricky to get right). Probably a good place to start is working out what pieces can be moved into separate libraries with e.g. a coherent layering story, thinking about how they might be reused in other tools etc.

Oops, sorry for the noise.

In D145228#4170826, @DmitryPolukhin wrote:

In D145228#4170792, @sammccall wrote:

The install target for clang distributes the clangd static libs

I don't think this was ever intended, looks like an accidental side-effect of using LLVM's many cmake macros.
Can we fix this instead?

Why not allow people building custom clangd outside of LLVM repo?

IIUC we want to link some extra targets into the clangd binary in order to provide extra clang-tidy checks, but *don't* want to customize the binary further.
(The latter can be useful, but this interface isn't sufficient for it).

The install target for clang distributes the clangd static libs

It seems pretty weird to me that the two child edges from functiondecl -> coroutinebodystmt -> compoundstmt are both called "body".

I wanted to share this to get some initial thoughts as we'd discussed debuggability a while ago.
As mentioned I'd at least want to get a reasonable HTML report before moving forward with it though.

Still LG

Fix colors, log source

Example textual log (though better with colors):

=== Beginning data flow analysis ===
int target(int x) {
    while (x > 0)
        {
            --x;
        }
    return x + 1;
}

A little more docs

The overloading impl is a little surprising to me.
I was assuming that object would always win over function (or that it would be disallowed to combine the two).
I think that is simpler to implement, as you know in advance what type of expansion you're attempting.
But what you have isn't that much more complicated, and the extra flexibility may prove useful.

Thanks, this all looks good, at least as far as I understand it! The two-passes-over-the-whole-file approach is easier for me to follow, too.

Looks pretty good. I think we can build the main file & preamble in parallel. Otherwise nits.

LG thanks!

It's happening!

Based on offline discussion, doubling down on checking in generated files to make these easy/possible to consume where they're needed.

Add comments to schema.yaml.
Add tests that generated files are up-to-date, and script to regenerate
Rename doc.md -> documentation.md

Only serious concern is getPreviousToken().

Code looks reasonable, but I don't understand why the changes are being made - can you explain/link to a bug in the commit message?

The code looks good, but I have a very hard time following the tests.

It looks like this fixes up the location only of diagnostics attached to particular directives (#include) based on some "deep" idea about the content of the directive (the spelled header name).

I can't understand from the description, code, or testcases what problem this is fixing. Can you clarify, ideally by improving the testcases?

oops, helps if I run tests on the right branch

In D142214#4076245, @foad wrote:

In D142214#4076194, @sammccall wrote:

TL;DR: this doesn't work in C++20, maybe revert?

I'm surprised the aggregate init works given the deleted copy ctor here, but if it works, it works I guess *shrug*

The aggregate init works up to C++17, but not in C++20.
(The rules change from "no user-provided, inherited, or explicit constructors" to "no user-declared or inherited constructors").

I think the current status of building LLVM in C++20 mode is "theoretically supported, but no stable buildbot yet". It's being worked on.

Since C++20 compatibility is something we'll need to do in the medium-term, and incompatible constructs make building a good bot harder in the short-term, I'd reluctantly suggest reverting this.

(Eliminating the aggregate init or finding an alternative technique to break copy/move also seem fine, starting with a revert would still be nice if it's not trivial)

Interesting. Is there any easy way I can test building in C++20 mode on my own machine?

TL;DR: this doesn't work in C++20, maybe revert?

LG, just nits really!

To offer the opposing argument: if DeclResult is just a bad idea, then using it consistently/right might be worse than using it as little as possible.

LG, thanks!

Thanks! This all looks pretty good to me apart from the complexity around comment markers, detecting which comment markers to use for which language etc.
I'd really like if you could either: