[PATCH] D101569: [LLD] [COFF] Fix automatic export of symbols from LTO objects

[Reid Kleckner via Phabricator via llvm-commits]

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

lgtm, with a nit



================
Comment at: lld/COFF/InputFiles.cpp:1050
+  FakeSection(int c) { section.Characteristics = c; }
+  const coff_section *operator&() const { return §ion; }
+
----------------
Wouldn't it be simpler to make the field public, and take the address of the field in the constructor below?


================
Comment at: lld/COFF/InputFiles.cpp:1047
+public:
+  FakeSection(int C) {
+    Characteristics = C;
----------------
mstorsjo wrote:
> mstorsjo wrote:
> > rnk wrote:
> > > Can this be marked constexpr? We want to avoid any dynamic initialization if possible.
> > Hmm, I'll have to see.
> Doesn't seem like it's doable to make it constexpr in this form, it'd have to be e.g. `constexpr FakeSection(int c) : section(<initializer expression>) {}`, it seems (at least in C++11).
> 
> The most straightforward would be C99 style designated intializers, i.e `coff_section fakeSection = { .Characteristics = 42; };`, but that's only available in C++20. And I'd rather not enumerate the members by hand to create something like `coff_section fakeSection = { 0, 0, 0, 0, 0, 0, importantValue }`.
> 
> Also, `coff_section` consists of `support::ulittle16_t`, which don't seem to be constexpr'able, so I'm not sure if it'd even work like that in C++20?
> 
> (This is all kinda roundabout, when what we really want just is a couple static bytes laid out in a specific way, but accessible for the caller via the `coff_section` class.)
Oh well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101569