aprantl added inline comments.
================ Comment at: lib/CodeGen/MacroPPCallbacks.cpp:62 + : DebugInfo(DI), PP(PP), FirstInclude(false), FirstIncludeDone(false), + CommandIncludeFiles(0), SkipFiles(2) { + Parents.push_back(nullptr); ---------------- aprantl wrote: > Comment why SkipFiles is initialized to 2? Thanks. I think this comment should now go into the header and be promoted to the Docygen comment for the CommandIncludeFiles. Also consider doing `CommandIncludeFiles = 0` in the class definition. ================ Comment at: lib/CodeGen/MacroPPCallbacks.cpp:31 + MacroInfo::arg_iterator AI = MI.arg_begin(), E = MI.arg_end(); + for (; AI + 1 != E; ++AI) { + Name << (*AI)->getName(); ---------------- std::next() ? ================ Comment at: lib/CodeGen/MacroPPCallbacks.cpp:44 + if (MI.isGNUVarargs()) + Name << "..."; // #define foo(x...) + ---------------- LLVM style prefers comments to be full sentences and above the code. ================ Comment at: lib/CodeGen/MacroPPCallbacks.cpp:83 + break; + // Fall though! + case MainFileScope: ---------------- I think there is an LLVM_FALLTHROUGH macro (or something to that end) ================ Comment at: lib/CodeGen/MacroPPCallbacks.cpp:154 + StringRef SearchPath, StringRef RelativePath, const Module *Imported) { + // Only care about "include" directives. + if (!IncludeTok.is(tok::identifier)) ---------------- #include as opposed to #import? If so, why? ================ Comment at: lib/CodeGen/ModuleBuilder.cpp:68 SmallVector<CXXMethodDecl *, 8> DeferredInlineMethodDefinitions; + CGDebugInfo *DebugInfoRef; ---------------- What does Ref stand for? https://reviews.llvm.org/D16135 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits