[PATCH] D16077: DIBuilder support DI Macro creation

[Adrian Prantl via Phabricator via llvm-commits]

aprantl accepted this revision.
aprantl added a comment.
This revision is now accepted and ready to land.

Thanks! LGTM with outstanding comments addressed.



================
Comment at: lib/IR/DIBuilder.cpp:210
+  auto *MF = DIMacroFile::getTemporary(VMContext, dwarf::DW_MACINFO_start_file,
+                                       LineNumber, File, Elements).release();
+  // Add the new DIFileMacro to the macro per parent map as a macro.
----------------
aaboud wrote:
> aprantl wrote:
> > Why does this need to be a temporary node? Can there be cycles?
> The temporary node is needed because at this point we do not know the Elements of the DIMacroFile. So, we cannot create the DIMacroFile as Unique at this point (it is not final yet).
> 
> So, there is no Cycles, but temporary nodes are still needed, and it was you who suggested this solution few months ago :)
> 
> By the way, 'Elements' will always be an empty array passed to this function, but i was not sure if I should remove it or keep it. What do you think? 
I think we should remove it then and explain how it is being populated in the doxygen comment.


https://reviews.llvm.org/D16077