[PATCH] D16135: Macro Debug Info support in Clang
Richard Smith via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 31 13:37:32 PST 2017
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
More information about the cfe-commits
mailing list