[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

steakhal added inline comments. Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:149 + if (!CI) +CI = new CompilerInstance(); + thieta wrote: > steakhal wrote: > > What takes the ownership of `CI`? When is it deleted? > I don't think

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta added inline comments. Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:42 + : Ctx(Context), SM(Context.getSourceManager()) { +CurrentFileName = astFilePath.str(); + } steakhal wrote: > Why is this not initialized in the

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

steakhal added a comment. Sorry about my delayed response. I was busy. I've left a couple comments inline. Nothing serious. Thanks for the patch! Comment at: clang/docs/ReleaseNotes.rst:593 + +- clang-extdef-mapping now accepts .ast files as input. This is faster than to +

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGe6ff553979e8: [clang-extdef-mapping] Directly process .ast files (authored by thieta). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta updated this revision to Diff 442211. thieta marked 3 inline comments as done. thieta added a comment. Uppercase all variables Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128704/new/ https://reviews.llvm.org/D128704 Files:

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta added a comment. Ran some internal benchmarks on 1570 files (C and C++ mixed but much more C++ than C): Running clang-extdef-mapping on the source files took: 268s Running clang-extdef-mapping on the AST: 102s That's quite a large speed up if you already need to generate the AST files.

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

martong accepted this revision. martong added a comment. This revision is now accepted and ready to land. Thanks for the update! LGTM (with very minor naming nits). Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:146 + +static bool HandleAST(StringRef

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta marked an inline comment as done. thieta added a comment. Thanks for the review! I uploaded a new version addressing all (I think) of your feedback and added a release note. The help text is a bit weird since it's using a commonoptionsparser we can't really change the text to add `` as

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta updated this revision to Diff 441930. thieta marked 6 inline comments as done. thieta added a comment. Addressed review feedback. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128704/new/ https://reviews.llvm.org/D128704 Files:

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

martong added a comment. Thanks, this is a great addition! - Please use Capitalized names for all the variables (as per the LLVM coding standards suggests here ). - Could you please update the

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta updated this revision to Diff 441279. thieta added a comment. Clean-ups and error handling. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128704/new/ https://reviews.llvm.org/D128704 Files: clang/test/Analysis/func-mapping-test.cpp

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta added inline comments. Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:136 + FileManager fm(CI.getFileSystemOpts()); + SmallString<128> absPath(astPath); + fm.makeAbsolutePath(absPath); thieta wrote: > Pretty sure 128 is wrong here -

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta added inline comments. Comment at: clang/tools/clang-extdef-mapping/ClangExtDefMapGen.cpp:122 + + CompilerInstance CI; + Not sure if I can just create a compilerinstance here and if I should feed it anymore data then I already do.

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta updated this revision to Diff 440558. thieta added a comment. Add test. Just repurpose the same test we already have but add a step to generate ast first and then pushing that through extdef-mapping. It should always produce the same result. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta added a comment. This still needs tests - but I wanted to get your early input on the approach here and what you all think. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128704/new/ https://reviews.llvm.org/D128704

[PATCH] D128704: [clang-extdef-mapping] Directly process .ast files

thieta created this revision. thieta added reviewers: steakhal, martong, NoQ. Herald added a subscriber: rnkovacs. Herald added a project: All. thieta requested review of this revision. Herald added a project: clang. When doing CTU analysis setup you pre-compile .cpp to .ast and then you run