aaboud marked 7 inline comments as done.
aaboud added a comment.
Thanks Adrian and Richard for the comments, I appreciate that.
I am uploading a new patch that address most of the comments.
Adrian, I answered some of your comments below.
Thanks,
Amjad
================
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();
----------------
aprantl wrote:
> std::next() ?
I am not that satisfied with this function, and I hope we can do it in a better
way.
This function is based on a code I took from this a static function in file
"lib\Frontend\PrintPreprocessedOutput.cpp":
```
/// PrintMacroDefinition - Print a macro definition in a form that will be
/// properly accepted back as a definition.
static void PrintMacroDefinition(const IdentifierInfo &II, const MacroInfo &MI,
Preprocessor &PP, raw_ostream &OS) {
```
================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:44
+ if (MI.isGNUVarargs())
+ Name << "..."; // #define foo(x...)
+
----------------
aprantl wrote:
> LLVM style prefers comments to be full sentences and above the code.
As I said above, this code is taken from
"lib\Frontend\PrintPreprocessedOutput.cpp"
Also, I ran clang-format on it, and it did not suggest to change this line.
But, I do not mind fixing this, if it is the preferred comment style.
================
Comment at: lib/CodeGen/MacroPPCallbacks.cpp:154
+ StringRef SearchPath, StringRef RelativePath, const Module *Imported) {
+ // Only care about "include" directives.
+ if (!IncludeTok.is(tok::identifier))
----------------
aprantl wrote:
> #include as opposed to #import? If so, why?
I am not familiar with "#import" keyword.
1. Is it handled differently by clang that #include?
2. Do we need to generate macro debug info for it?
The reason I have this switch below, is that I am not sure if we call this
"InclusionDirective" callback for identifiers that are not "#include".
However, I see no harm of updating the LastHashLoc every time we enter this
function. The value will not be used unless it is set just before calling
"FileChanged"callback.
================
Comment at: lib/CodeGen/ModuleBuilder.cpp:68
SmallVector<CXXMethodDecl *, 8> DeferredInlineMethodDefinitions;
+ CGDebugInfo *DebugInfoRef;
----------------
aprantl wrote:
> What does Ref stand for?
Ref - stands for reference, I think I do not need the "Ref" prefix for this
member, only for the get function.
But, I will follow Richards suggestion that will eliminate the need for this
member.
https://reviews.llvm.org/D16135
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
