[PATCH] D16077: DIBuilder support DI Macro creation

[Amjad Aboud via Phabricator via llvm-commits]

aaboud added a comment.

Thanks Adrian for the comments, I will do the changes as needed and upload a new patch.
I have one answer below.



================
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.
----------------
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? 


https://reviews.llvm.org/D16077