[PATCH] D29609: [dsymutil] Follow dependencies during ODR uniquing

[Alexander Shaposhnikov via Phabricator via llvm-commits]

alexshap added inline comments.


================
Comment at: tools/dsymutil/DwarfLinker.cpp:2197
+                        TF_ParentWalk | TF_Keep | TF_DependencyWalk | ODRFlag, 
+                        false);
+      AncestorIdx = CU.getInfo(AncestorIdx).ParentIdx;
----------------
friss wrote:
> aprantl wrote:
> > either /*DIEODRSeen=*/false or use an enum.
> Is it possible to make it an additional TF_* enum and not add a function argument.
it is, there is a room there, but that looks more complicated because those flags are changed by shouldKeep* functions.

If my understanding is correct, the logic can be summarized as follows (please, correct me if I am wrong):

1. there is a recursive walk over DIEs where we mark some of them as Kept (taking into account ODR uniquing). Technically this walk is sequence of "interleaved" calls of lookForDIEsToKeep and keepDIEAndDependencies.
2. lookForDIEsToKeep calls keepDIEAndDependencies and then follows the children
3. keepDIEAndDependencies sets the actual flag "Keep" to true and then follows the attributes

before my change: in keepDIEAndDependencies if a DIE "referenced via attibutes" is considered "ODR seen" we didn't check the further dependencies of that DIE and just skipped it
after my change: keepDIEAndDependencies if a DIE "referenced via attibute" 
is considered "ODR seen" we still don't mark it as "Kept" but follow the attributes and check the dependencies.

the changes in lookForDIEsToKeep are necessary because it's recursively called from keepDIEAndDependencies (lines 2243 - 2245) and we need to propagate this flag there. 

Please, feel free to correct me if i am mistaken / missing smth.






Repository:
  rL LLVM

https://reviews.llvm.org/D29609