[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
This revision was automatically updated to reflect the committed changes.
Closed by commit rC332578: [libclang] Allow skipping function bodies in
preamble only (authored by yvvan, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D45815?vs=145648&id=147253#toc
Repository:
rC Clang
https://reviews.llvm.org/D45815
Files:
include/clang-c/Index.h
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
test/Parser/skip-function-bodies.h
test/Parser/skip-function-bodies.mm
tools/c-index-test/c-index-test.c
tools/libclang/CIndex.cpp
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1268,13 +1268,13 @@
/// \returns If the precompiled preamble can be used, returns a newly-allocated
/// buffer that should be used in place of the main file when doing so.
/// Otherwise, returns a NULL pointer.
-std::unique_ptr
-ASTUnit::getMainBufferWithPrecompiledPreamble(
-std::shared_ptr PCHContainerOps,
-const CompilerInvocation &PreambleInvocationIn,
-IntrusiveRefCntPtr VFS, bool AllowRebuild,
-unsigned MaxLines) {
- auto MainFilePath =
+std::unique_ptr
+ASTUnit::getMainBufferWithPrecompiledPreamble(
+std::shared_ptr PCHContainerOps,
+CompilerInvocation &PreambleInvocationIn,
+IntrusiveRefCntPtr VFS, bool AllowRebuild,
+unsigned MaxLines) {
+ auto MainFilePath =
PreambleInvocationIn.getFrontendOpts().Inputs[0].getFile();
std::unique_ptr MainFileBuffer =
getBufferForFileHandlingRemapping(PreambleInvocationIn, VFS.get(),
@@ -1335,15 +1335,24 @@
&NewPreambleDiagsStandalone);
// We did not previously compute a preamble, or it can't be reused anyway.
-SimpleTimer PreambleTimer(WantTiming);
-PreambleTimer.setOutput("Precompiling preamble");
-
-llvm::ErrorOr NewPreamble = PrecompiledPreamble::Build(
-PreambleInvocationIn, MainFileBuffer.get(), Bounds, *Diagnostics, VFS,
-PCHContainerOps, /*StoreInMemory=*/false, Callbacks);
-if (NewPreamble) {
- Preamble = std::move(*NewPreamble);
- PreambleRebuildCounter = 1;
+SimpleTimer PreambleTimer(WantTiming);
+PreambleTimer.setOutput("Precompiling preamble");
+
+const bool PreviousSkipFunctionBodies =
+PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies;
+if (SkipFunctionBodies == SkipFunctionBodiesScope::Preamble)
+ PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies = true;
+
+llvm::ErrorOr NewPreamble = PrecompiledPreamble::Build(
+PreambleInvocationIn, MainFileBuffer.get(), Bounds, *Diagnostics, VFS,
+PCHContainerOps, /*StoreInMemory=*/false, Callbacks);
+
+PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies =
+PreviousSkipFunctionBodies;
+
+if (NewPreamble) {
+ Preamble = std::move(*NewPreamble);
+ PreambleRebuildCounter = 1;
} else {
switch (static_cast(NewPreamble.getError().value())) {
case BuildPreambleError::CouldntCreateTempFile:
@@ -1688,13 +1697,13 @@
std::shared_ptr PCHContainerOps,
IntrusiveRefCntPtr Diags, StringRef ResourceFilesPath,
bool OnlyLocalDecls, bool CaptureDiagnostics,
-ArrayRef RemappedFiles, bool RemappedFilesKeepOriginalName,
-unsigned PrecompilePreambleAfterNParses, TranslationUnitKind TUKind,
-bool CacheCodeCompletionResults, bool IncludeBriefCommentsInCodeCompletion,
-bool AllowPCHWithCompilerErrors, bool SkipFunctionBodies,
-bool SingleFileParse, bool UserFilesAreVolatile, bool ForSerialization,
-llvm::Optional ModuleFormat, std::unique_ptr *ErrAST,
-IntrusiveRefCntPtr VFS) {
+ArrayRef RemappedFiles, bool RemappedFilesKeepOriginalName,
+unsigned PrecompilePreambleAfterNParses, TranslationUnitKind TUKind,
+bool CacheCodeCompletionResults, bool IncludeBriefCommentsInCodeCompletion,
+bool AllowPCHWithCompilerErrors, SkipFunctionBodiesScope SkipFunctionBodies,
+bool SingleFileParse, bool UserFilesAreVolatile, bool ForSerialization,
+llvm::Optional ModuleFormat, std::unique_ptr *ErrAST,
+IntrusiveRefCntPtr VFS) {
assert(Diags.get() && "no DiagnosticsEngine was provided");
SmallVector StoredDiagnostics;
@@ -1721,13 +1730,14 @@
PPOpts.AllowPCHWithCompilerErrors = AllowPCHWithCompilerErrors;
PPOpts.SingleFileParseMode = SingleFileParse;
- // Override the resources path.
- CI->getHeaderSearchOpts().ResourceDir = ResourceFilesPath;
-
- CI->getFrontendOpts().SkipFunctionBodies = SkipFunctionBodies;
-
- if (ModuleFormat)
-CI->getHeaderSearchOpts().ModuleFormat = ModuleFormat.getValue();
+ // Override the resources path.
+ CI->getHeaderSearchOpts().ResourceDir = ResourceFilesPath;
+
+ CI->getFrontendOpts().SkipFunctionBodies =
+ SkipFunctionBodies == SkipFunctionBodiesScope::PreambleAndMainFile;
+
+ if (ModuleFormat)
+CI->getHeaderSearchOpts().ModuleFormat = ModuleFormat.getValue();
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM Repository: rC Clang https://reviews.llvm.org/D45815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
nik added inline comments. Comment at: include/clang/Frontend/ASTUnit.h:370 + IntrusiveRefCntPtr VFS, + SkipFunctionBodiesScope SkipFunctionBodiesScp = + SkipFunctionBodiesScope::None, ilya-biryukov wrote: > NIT: Maybe keep the name `SkipFunctionBodies`? It seems that putting `Scp` at > the end does not add any clarity. IIRC I've done that to better distinguish it from e.g. "getFrontendOpts().SkipFunctionBodies", but yes, they are not so often used together, removed "Scp" suffix. Comment at: include/clang/Frontend/ASTUnit.h:831 ArrayRef RemappedFiles = None, + SkipFunctionBodiesScope SkipFunctionBodiesScp = + SkipFunctionBodiesScope::None, ilya-biryukov wrote: > This parameter seems slightly out of place. > The scope of skipped function bodies should be a property of the whole > `ASTUnit`, changing it on every reparse would mean more complexity and > wouldn't be used anyway. > > I suggest storing a field of type `SkipFunctionBodiesScope` instead of adding > parameters to `Reparse` and `LoadFromCompilerInvocation`. Agree, this makes the change significantly shorter :) Repository: rC Clang https://reviews.llvm.org/D45815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
nik updated this revision to Diff 145648.
nik marked 3 inline comments as done.
nik edited the summary of this revision.
nik added a comment.
Addressed comments.
Repository:
rC Clang
https://reviews.llvm.org/D45815
Files:
include/clang-c/Index.h
include/clang/Frontend/ASTUnit.h
lib/Frontend/ASTUnit.cpp
test/Parser/skip-function-bodies.h
test/Parser/skip-function-bodies.mm
tools/c-index-test/c-index-test.c
tools/libclang/CIndex.cpp
Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -3380,9 +3380,15 @@
= options & CXTranslationUnit_CacheCompletionResults;
bool IncludeBriefCommentsInCodeCompletion
= options & CXTranslationUnit_IncludeBriefCommentsInCodeCompletion;
- bool SkipFunctionBodies = options & CXTranslationUnit_SkipFunctionBodies;
bool SingleFileParse = options & CXTranslationUnit_SingleFileParse;
bool ForSerialization = options & CXTranslationUnit_ForSerialization;
+ SkipFunctionBodiesScope SkipFunctionBodies = SkipFunctionBodiesScope::None;
+ if (options & CXTranslationUnit_SkipFunctionBodies) {
+SkipFunctionBodies =
+(options & CXTranslationUnit_LimitSkipFunctionBodiesToPreamble)
+? SkipFunctionBodiesScope::Preamble
+: SkipFunctionBodiesScope::PreambleAndMainFile;
+ }
// Configure the diagnostics.
IntrusiveRefCntPtr
Index: tools/c-index-test/c-index-test.c
===
--- tools/c-index-test/c-index-test.c
+++ tools/c-index-test/c-index-test.c
@@ -82,6 +82,8 @@
options |= CXTranslationUnit_CreatePreambleOnFirstParse;
if (getenv("CINDEXTEST_KEEP_GOING"))
options |= CXTranslationUnit_KeepGoing;
+ if (getenv("CINDEXTEST_LIMIT_SKIP_FUNCTION_BODIES_TO_PREAMBLE"))
+options |= CXTranslationUnit_LimitSkipFunctionBodiesToPreamble;
return options;
}
Index: test/Parser/skip-function-bodies.mm
===
--- test/Parser/skip-function-bodies.mm
+++ test/Parser/skip-function-bodies.mm
@@ -1,4 +1,4 @@
-// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source all %s | FileCheck %s
+#include "skip-function-bodies.h"
class A {
class B {};
@@ -27,6 +27,7 @@
class K {};
}
+// RUN: env CINDEXTEST_SKIP_FUNCTION_BODIES=1 c-index-test -test-load-source all %s | FileCheck %s
// CHECK: skip-function-bodies.mm:3:7: ClassDecl=A:3:7 (Definition) Extent=[3:1 - 14:2]
// CHECK: skip-function-bodies.mm:4:9: ClassDecl=B:4:9 (Definition) Extent=[4:3 - 4:13]
// CHECK: skip-function-bodies.mm:6:1: CXXAccessSpecifier=:6:1 (Definition) Extent=[6:1 - 6:8]
@@ -43,3 +44,13 @@
// CHECK-NOT: skip-function-bodies.mm:21:11: TypeRef=class A:3:7 Extent=[21:11 - 21:12]
// CHECK: skip-function-bodies.mm:26:6: FunctionDecl=J:26:6 Extent=[26:1 - 26:9]
// CHECK-NOT: skip-function-bodies.mm:27:9: ClassDecl=K:27:9 (Definition) Extent=[27:3 - 27:13]
+
+// RUN: env CINDEXTEST_EDITING=1 \
+// RUN: CINDEXTEST_CREATE_PREAMBLE_ON_FIRST_PARSE=1 \
+// RUN: CINDEXTEST_SKIP_FUNCTION_BODIES=1 \
+// RUN: CINDEXTEST_LIMIT_SKIP_FUNCTION_BODIES_TO_PREAMBLE=1 \
+// RUN: c-index-test -test-load-source all %s | FileCheck -check-prefix=CHECK-PREAMBLE %s
+// CHECK-PREAMBLE: skip-function-bodies.h:1:5: FunctionDecl=header1:1:5 Extent=[1:1 - 1:19]
+// CHECK-PREAMBLE-NOT: skip-function-bodies.h:2:3: ReturnStmt= Extent=[2:3 - 2:11]
+// CHECK-PREAMBLE: skip-function-bodies.mm:8:12: StructDecl=C:8:12 (Definition) Extent=[8:5 - 10:6]
+// CHECK-PREAMBLE: skip-function-bodies.mm:9:12: CXXMethod=d:9:12 (Definition) Extent=[9:7 - 9:18]
Index: test/Parser/skip-function-bodies.h
===
--- /dev/null
+++ test/Parser/skip-function-bodies.h
@@ -0,0 +1,3 @@
+int header1(int t) {
+ return t;
+}
Index: lib/Frontend/ASTUnit.cpp
===
--- lib/Frontend/ASTUnit.cpp
+++ lib/Frontend/ASTUnit.cpp
@@ -1271,7 +1271,7 @@
std::unique_ptr
ASTUnit::getMainBufferWithPrecompiledPreamble(
std::shared_ptr PCHContainerOps,
-const CompilerInvocation &PreambleInvocationIn,
+CompilerInvocation &PreambleInvocationIn,
IntrusiveRefCntPtr VFS, bool AllowRebuild,
unsigned MaxLines) {
auto MainFilePath =
@@ -1338,9 +1338,18 @@
SimpleTimer PreambleTimer(WantTiming);
PreambleTimer.setOutput("Precompiling preamble");
+const bool PreviousSkipFunctionBodies =
+PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies;
+if (SkipFunctionBodies == SkipFunctionBodiesScope::Preamble)
+ PreambleInvocationIn.getFrontendOpts().SkipFunctionBodies = true;
+
llvm::ErrorOr NewPreamble = PrecompiledPreamble::Build(
PreambleInvocationIn, MainFileBuffer.get(), Bounds, *Diagnostics, VFS,
PCHContainerOps, /*StoreInMemory=*/false, Callbacks);
+
+PreambleInvoc
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
ilya-biryukov added a comment. Sorry for the delay Repository: rC Clang https://reviews.llvm.org/D45815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
ilya-biryukov added inline comments.
Comment at: include/clang/Frontend/ASTUnit.h:104
+ enum class SkipFunctionBodiesScope { None, MainFileAndPreamble, Preamble };
+
Maybe move this out of `ASTUnit`? Would allow removing the first qualifier in
usages outside the class itself (i.e.
`ASTUnit::SkipFunctionBodiesScope::Preamble` will be shorter!)
Comment at: include/clang/Frontend/ASTUnit.h:370
+ IntrusiveRefCntPtr VFS,
+ SkipFunctionBodiesScope SkipFunctionBodiesScp =
+ SkipFunctionBodiesScope::None,
NIT: Maybe keep the name `SkipFunctionBodies`? It seems that putting `Scp` at
the end does not add any clarity.
Comment at: include/clang/Frontend/ASTUnit.h:831
ArrayRef RemappedFiles = None,
+ SkipFunctionBodiesScope SkipFunctionBodiesScp =
+ SkipFunctionBodiesScope::None,
This parameter seems slightly out of place.
The scope of skipped function bodies should be a property of the whole
`ASTUnit`, changing it on every reparse would mean more complexity and wouldn't
be used anyway.
I suggest storing a field of type `SkipFunctionBodiesScope` instead of adding
parameters to `Reparse` and `LoadFromCompilerInvocation`.
Repository:
rC Clang
https://reviews.llvm.org/D45815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
nik added a comment. Do I miss something? I've uploaded a new diff/version and state is still "(X) Requested Changes to Prior Diff". Waiting for review. Repository: rC Clang https://reviews.llvm.org/D45815 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
nik added a comment.
Trying to format the diff in the previous comment:
--- a/lib/Parse/ParseCXXInlineMethods.cpp
+++ b/lib/Parse/ParseCXXInlineMethods.cpp
@@ -102,9 +102,14 @@ NamedDecl
*Parser::ParseCXXInlineMethodDef(AccessSpecifier AS,
}
if (SkipFunctionBodies != SkipFunctionBodiesKind::None &&
+ TemplateInfo.Kind == ParsedTemplateInfo::NonTemplate &&
+ !isa(getCurrentClass().TagOrTemplate) &&
+ !isa(getCurrentClass().TagOrTemplate)
&&
+ !isa(
+ getCurrentClass().TagOrTemplate) &&
(!FnD || Actions.canSkipFunctionBody(FnD)) &&
trySkippingFunctionBody()) {
Actions.ActOnSkippedFunctionBody(FnD);
Repository:
rC Clang
https://reviews.llvm.org/D45815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
nik added a comment.
OK, to skip all function bodies in the preamble except for template functions
and functions within a template class, I've amended the previous diff with:
- a/lib/Parse/ParseCXXInlineMethods.cpp +++
b/lib/Parse/ParseCXXInlineMethods.cpp @@ -102,9 +102,14 @@ NamedDecl
*Parser::ParseCXXInlineMethodDef(AccessSpecifier AS, }
if (SkipFunctionBodies != SkipFunctionBodiesKind::None && +
TemplateInfo.Kind == ParsedTemplateInfo::NonTemplate && +
!isa(getCurrentClass().TagOrTemplate) && +
!isa(getCurrentClass().TagOrTemplate) && +
!isa( +
getCurrentClass().TagOrTemplate) && (!FnD || Actions.canSkipFunctionBody(FnD))
&& trySkippingFunctionBody()) { Actions.ActOnSkippedFunctionBody(FnD);
I'm not sure whether this covers all cases, but here are the timing in relation
to others:
Reparse for skipping all function bodies: 0.2s
Reparse for skipping all function bodies except template functions (previous
patch set, not handling function bodies within template classes): 0.27s
Reparse for skipping all function bodies with the diff above (fixing another
case + ignoring function bodies in templates): 0.44s
Reparse without skipping any function bodies in the preamble: 0.51s
As I said, I'm not sure whether this diff covered all cases, but it can get
only more worse than 0.44s :) That's enough for me to stop here.
Repository:
rC Clang
https://reviews.llvm.org/D45815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D45815: [libclang] Allow skipping function bodies in preamble only
nik added inline comments.
Comment at: lib/Parse/ParseCXXInlineMethods.cpp:104
- if (SkipFunctionBodies && (!FnD || Actions.canSkipFunctionBody(FnD)) &&
- trySkippingFunctionBody()) {
+ if (SkipFunctionBodies != SkipFunctionBodiesKind::None &&
+ (!FnD || Actions.canSkipFunctionBody(FnD)) && trySkippingFunctionBody())
{
ilya-biryukov wrote:
> Can't this be a template too? Do we need to check for it here?
Correct. Here is also the point were we would check for the parent class being
a template.
Repository:
rC Clang
https://reviews.llvm.org/D45815
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
