aaron.ballman added inline comments.
Comment at: clang-tidy/ClangTidy.h:230-232
+/// \param StoreCheckProfile If provided, and EnableCheckProfile is true,
+/// the profile will not be outputted to the stderr, but instead will be stored
+/// as JSON file in the specified directory
lebedev.ri added inline comments.
Comment at: clang-tidy/tool/ClangTidyMain.cpp:342-347
+if (!llvm::sys::fs::exists(AbsolutePath)) {
+ // If the destination prefix does not exist, don't try to use
real_path().
+ return AbsolutePath;
+}
+SmallString<256> des
lebedev.ri updated this revision to Diff 149610.
lebedev.ri marked 6 inline comments as done.
lebedev.ri added a comment.
Rebased.
Addressed review notes.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46602
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/ClangTidy.h
clang-tid
aaron.ballman added inline comments.
Comment at: clang-tidy/ClangTidyProfiling.cpp:72
+ if (EC) {
+llvm::errs() << "Error opening output file'" << Storage->StoreFilename
+ << "': " << EC.message() << "\n";
Missing a whitespace before the quot
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1116973, @george.karpenkov wrote:
> LGTM with a nit to me
Thank you!
I suspect that at least temporarily we will end up with two different tooling
sets
to further post-process these results, since i really love to write bicycles a
george.karpenkov accepted this revision.
george.karpenkov added a comment.
LGTM with a nit to me, but maybe clang-tidy code owners would need to sign that
off as well.
Comment at: clang-tidy/ClangTidy.h:232
+ bool EnableCheckProfile = false,
+
lebedev.ri added a comment.
Ping.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46602
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
lebedev.ri added inline comments.
Comment at: clang-tidy/tool/ClangTidyMain.cpp:188
+format to stderr. When this option is passed,
+these per-TU profiles are instead stored as YAML.)"),
+ cl::value_desc("prefix"),
Quux
Quuxplusone added inline comments.
Comment at: clang-tidy/tool/ClangTidyMain.cpp:188
+format to stderr. When this option is passed,
+these per-TU profiles are instead stored as YAML.)"),
+ cl::value_desc("prefix"),
Dri
lebedev.ri updated this revision to Diff 147120.
lebedev.ri marked 2 inline comments as done.
lebedev.ri added a comment.
Rename getter/setter functions.
Thank you for taking a look!
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46602
Files:
clang-tidy/ClangTidy.cpp
clang
alexfh added a comment.
A couple of comments from a cursory look. I'll try to look closer later this
week.
Comment at: clang-tidy/ClangTidyDiagnosticConsumer.h:173
+ /// \brief Control storage of profile date.
+ void setStoreProfile(StringRef ProfilePrefix);
+ llvm::Optiona
lebedev.ri updated this revision to Diff 147071.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
- Rebased
- Slightly refactor how the profile storage params are computed, move that into
`ClangTidyProfiling::StorageParams` helper struct.
- Store timestamp in the json t
lebedev.ri added a project: clang-tools-extra.
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1098092, @alexfh wrote:
> In https://reviews.llvm.org/D46602#1097954, @lebedev.ri wrote:
>
> > How do i reflect that in tests? The output name will basically be random.
>
>
> Why random?
lebedev.ri updated this revision to Diff 146644.
lebedev.ri edited the summary of this revision.
lebedev.ri added subscribers: aaron.ballman, JonasToth.
lebedev.ri added a comment.
- Get rid of 'prefix elision'
- Don't use full source file name, only the filename
- Prefix that filename with ISO860
alexfh added a comment.
In https://reviews.llvm.org/D46602#1097954, @lebedev.ri wrote:
> In https://reviews.llvm.org/D46602#1097902, @alexfh wrote:
>
> > In https://reviews.llvm.org/D46602#1095980, @lebedev.ri wrote:
> >
> > > In https://reviews.llvm.org/D46602#1095960, @alexfh wrote:
> > >
> > >
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1097902, @alexfh wrote:
> In https://reviews.llvm.org/D46602#1095980, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D46602#1095960, @alexfh wrote:
> >
> > > In https://reviews.llvm.org/D46602#1092902, @lebedev.ri wrote:
> > >
>
alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.
In https://reviews.llvm.org/D46602#1095980, @lebedev.ri wrote:
> In https://reviews.llvm.org/D46602#1095960, @alexfh wrote:
>
> > In https://reviews.llvm.org/D46602#1092902, @lebedev.
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1095960, @alexfh wrote:
> In https://reviews.llvm.org/D46602#1092902, @lebedev.ri wrote:
>
> > In https://reviews.llvm.org/D46602#1092890, @alexfh wrote:
> >
> > > Roman, it looks to me that a simpler storage scheme would be sufficien
alexfh added a comment.
In https://reviews.llvm.org/D46602#1092902, @lebedev.ri wrote:
> In https://reviews.llvm.org/D46602#1092890, @alexfh wrote:
>
> > Roman, it looks to me that a simpler storage scheme would be sufficient.
> > For example, MMDDhhmmss-InputFileName.cpp.csv.
> > Main thin
lebedev.ri updated this revision to Diff 145995.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
- Make json less flat, store source filename in it.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D46602
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/C
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1092902, @lebedev.ri wrote:
> In https://reviews.llvm.org/D46602#1092890, @alexfh wrote:
>
> > Roman, it looks to me that a simpler storage scheme would be sufficient.
> > For example, MMDDhhmmss-InputFileName.cpp.csv.
> > Main
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1092890, @alexfh wrote:
> Roman, it looks to me that a simpler storage scheme would be sufficient. For
> example, MMDDhhmmss-InputFileName.cpp.csv.
> Main things are:
>
> 1. include a timestamp, so there's no need to overwrite o
lebedev.ri added a comment.
In https://reviews.llvm.org/D46602#1092883, @alexfh wrote:
> In https://reviews.llvm.org/D46602#1092111, @rja wrote:
>
> > +1 for JSON
>
>
> Could you explain why would you use YAML or JSON for this? How are you going
> to use/process this data?
I personally don't h
alexfh added a comment.
Roman, it looks to me that a simpler storage scheme would be sufficient. For
example, MMDDhhmmss-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 direc
alexfh added a comment.
In https://reviews.llvm.org/D46602#1092111, @rja wrote:
> +1 for JSON
Could you explain why would you use YAML or JSON for this? How are you going to
use/process this data?
Repository:
rL LLVM
https://reviews.llvm.org/D46602
_
lebedev.ri added inline comments.
Comment at: docs/clang-tidy/index.rst:783
+ {
+"time.clang-tidy.readability-function-size.wall": 1.0421266555786133e+00,
+"time.clang-tidy.readability-function-size.user": 9.208840005421e-01,
Hmm, thinking about it a
lebedev.ri updated this revision to Diff 145906.
lebedev.ri edited the summary of this revision.
lebedev.ri added a comment.
Herald added a subscriber: llvm-commits.
- Produce valid-er YAML.
Repository:
rL LLVM
https://reviews.llvm.org/D46602
Files:
clang-tidy/ClangTidy.cpp
clang-tidy/Cl
lebedev.ri updated this revision to Diff 145901.
lebedev.ri retitled this revision from "[clang-tidy] Store checks profiling
info as CSV files" to "[clang-tidy] Store checks profiling info as YAML files".
lebedev.ri added reviewers: george.karpenkov, NoQ.
lebedev.ri added a comment.
- Deduplicate
28 matches
Mail list logo
