[PATCH] D62350: [wip] ELF: Create synthetic sections for loadable partitions.

Peter Collingbourne via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 28 11:40:44 PDT 2019


pcc marked an inline comment as done.
pcc added inline comments.


================
Comment at: lld/ELF/SyntheticSections.h:1127
+
+inline Partition &mainPartition() { return Partitions[0]; }
+
----------------
ruiu wrote:
> grimar wrote:
> > What about having one more global pointer instead.
> > i.e. after creating all of the partitions do:
> > 
> > ```
> > Partition *InM; //global, like `In`
> > 
> > ...
> > 
> > InM = &Partitions[0];
> > ```
> > 
> > And then use `InM` everywhere instead of `mainPartition`.
> > I wonder what others think though.
> > 
> Yeah. Looks like this function doesn't have to be a function. I thought for a while about what kind of name I'd choose, but I didn't come up with a good idea. Calling Partitions[0] as In might not be a bad idea. It's short, and we don't need to update all `In`s with `mainPartition()` which makes this patch much easier to read.
It won't work to name it `In` because we already have a variable named `In`. I think that we'll always have a variable for per-file synthetic sections because some sections (such as the static symbol table) will not be duplicated between partitions.

In an earlier version of this change I had a global variable
```
static Partition &Main = Partitions[0];
```
but this won't work now that `Partitions` is a vector and not an array. That's why I introduced the function.

I'm fine with the idea of bringing the variable back (as a pointer rather than a reference) and setting it once all partitions are created. I'll call it `Main` unless there are any strong objections to that name.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62350/new/

https://reviews.llvm.org/D62350





More information about the llvm-commits mailing list