nik added inline comments.

================
Comment at: include/clang/Frontend/ASTUnit.h:370
+      IntrusiveRefCntPtr<vfs::FileSystem> 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<RemappedFile> 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

Reply via email to