rsmith added inline comments.
================ Comment at: include/clang/CodeGen/ModuleBuilder.h:71 + /// + /// This methods can be called before initializing the CGDebugInfo calss. + /// But the returned value should not be used until after initialization. ---------------- Typo 'calss' ================ Comment at: include/clang/CodeGen/ModuleBuilder.h:72 + /// This methods can be called before initializing the CGDebugInfo calss. + /// But the returned value should not be used until after initialization. + /// It is caller responsibility to validate that the place holder was ---------------- What is the returned value in that case? A reference to a null pointer? Generally, I'm not particularly happy with this approach of exposing a reference to an internal pointer as the way of propagating the `CGDebugInfo` to the macro generator as needed. Instead, how about giving the `MacroPPCallbacks` object a pointer to the `CodeGenerator` itself, and having it ask the `CodeGenModule` for the debug info object when it needs it? ================ Comment at: include/clang/CodeGen/ModuleBuilder.h:73 + /// But the returned value should not be used until after initialization. + /// It is caller responsibility to validate that the place holder was + /// initialized before start using it. ---------------- caller -> the caller's ================ Comment at: include/clang/CodeGen/ModuleBuilder.h:74 + /// It is caller responsibility to validate that the place holder was + /// initialized before start using it. + CodeGen::CGDebugInfo *&CodeGenerator::getModuleDebugInfoRef(); ---------------- start using -> starting to use ================ Comment at: lib/CodeGen/MacroPPCallbacks.cpp:1 +//===--- MacroPPCallbacks.h -------------------------------------*- C++ -*-===// +// ---------------- Wrong file header (should be .cpp, and you don't need the "-*- C++ -*-" in a .cpp file. https://reviews.llvm.org/D16135 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
