I've fixed the comments and committed the patch myself. Hope that's fine by you.

LG with a couple of nits. Do you need someone to commit the patch for you?

In D36892#1187959, @JonasToth wrote:

@lebedev.ri and @alexfh i would change the tests in https://reviews.llvm.org/D48714 to use CHECK-NOTES. Is it ok, to commit this one?

For testing purposes, you could change a single line of hicpp-exception-baseclass.cpp to use the CHECK-NOTES. I do the rest :)

LG

LG

Still LG

Could you describe the specific problem you're solving and provide an example? As mentioned by others, a test would be very welcome as well.

LG

LG modulo outstanding comments.

Thank you for working on this!

FTR, this is http://llvm.org/PR38072

(removing from my dashboard)

Looks good with one comment.

As per the previous comment: I have no concerns as long as the documentation is updated and at least one existing test is changed to use this feature (see the list in the previous comment).

That's an awesome idea! Did you think about supporting automated fix-it suggestions?

Some patterns are covered by compiler diagnostics: https://godbolt.org/g/HvsjnP. Is there any benefit in re-implementing them?

LG

Please add a test.

• Addressed review comments.

LG

Ping.

Please take another look.

• Added a test.

In D47670#1135497, @george.karpenkov wrote:

@alexfh apologies, fixed.

In D45679#1132086, @JonasToth wrote:

It might be the case, that the test is run with -no-stdinc (or similar),
so the standard library is not available.

This commit seems to have introduced https://bugs.llvm.org/show_bug.cgi?id=37769. Could you take a look?

LG

LG

LG

LG

In D45177#1103774, @devnexen wrote:

In D45177#1103162, @alexfh wrote:

See https://bugs.llvm.org/show_bug.cgi?id=37503 for a test case.

I was unable to reproduce both FreeBSD and Linux. Plus my changes come after checkNonNull.

See https://bugs.llvm.org/show_bug.cgi?id=37503 for a test case.

This is reproducible in r332425.

This patch seems to cause an assertion failure:

LG

A couple of comments from a cursory look. I'll try to look closer later this week.

LG

LG

LG

LG

There are still outstanding comments.

In D46602#1097954, @lebedev.ri wrote:

In D46602#1097902, @alexfh wrote:

In D46602#1095980, @lebedev.ri wrote:

In D46602#1095960, @alexfh wrote:

In D46602#1092902, @lebedev.ri wrote:

In D46602#1092890, @alexfh wrote:

Roman, it looks to me that a simpler storage scheme would be sufficient. For example, YYYYMMDDhhmmss-InputFileName.cpp.csv.
Main things are:

1. include a timestamp, so there's no need to overwrite old results,

Of the input source file?

No, current timestamp, when each profile gets dumped.

Ah.
Then i don't understand this at all. Why would we want to do that, exactly?
Just so that we can avoid creating directory structure? Why do we want to avoid that?

If clang-tidy is invoked manually, a simpler naming scheme with less configuration options would be easier to use, in particular:

1. no the need to specify the -store-check-profile-elide-prefix= option;
2. it's easier to see all results (no need to use find, just ls /output/directory or ls /output/directory/20180514* to see today's results, for example);
3. no chance of filename collision, and thus no chance of losing older results by just running clang-tidy again.

How do i reflect that in tests? The output name will basically be random.

In D46159#1098018, @alexfh wrote:

In D46159#1098000, @pfultz2 wrote:

Is someone able to merge in my changes?

Will do.

In D46159#1098000, @pfultz2 wrote:

Is someone able to merge in my changes?

In D46602#1095980, @lebedev.ri wrote:

In D46602#1095960, @alexfh wrote:

In D46602#1092902, @lebedev.ri wrote:

In D46602#1092890, @alexfh wrote:

Roman, it looks to me that a simpler storage scheme would be sufficient. For example, YYYYMMDDhhmmss-InputFileName.cpp.csv.
Main things are:

1. include a timestamp, so there's no need to overwrite old results,

Of the input source file?

No, current timestamp, when each profile gets dumped.

Ah.
Then i don't understand this at all. Why would we want to do that, exactly?
Just so that we can avoid creating directory structure? Why do we want to avoid that?

In D46602#1092902, @lebedev.ri wrote:

In D46602#1092890, @alexfh wrote:

Roman, it looks to me that a simpler storage scheme would be sufficient. For example, YYYYMMDDhhmmss-InputFileName.cpp.csv.
Main things are:

1. include a timestamp, so there's no need to overwrite old results,

Of the input source file?

In D33844#1095862, @koldaniel wrote:

In D33844#1086565, @alexfh wrote:

Which comments do you think are still relevant?

I wonder why JSON? What kind of a tool do folks use to process the outputs of printJSONValue? Is there anything existing or is JSON used "just in case"? I personally use either spreadsheets, python or shell when I deal with this kind of data, and all three of them can work reasonably well with CSV (plus the data is pretty simple).

Roman, it looks to me that a simpler storage scheme would be sufficient. For example, YYYYMMDDhhmmss-InputFileName.cpp.csv. Main things are: 1. include a timestamp, so there's no need to overwrite old results, 2. include just the name of the file without any parent directories, 3. put all outputs into the same directory. This way we wouldn't have to create a directory structure and think about stripping a certain prefix (btw, utilities like patch just specify the number of path components to remove from the start, not the actual substring). WDYT?

In D46602#1092111, @rja wrote:

+1 for JSON