[PATCH] D89420: [lld-macho][easy] Fix segment max protection

Saleem Abdulrasool via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Oct 21 08:53:48 PDT 2020


compnerd added inline comments.


================
Comment at: lld/MachO/OutputSegment.cpp:36
+         "TODO: i386 has different maxProt requirements");
+  return initProt(name);
 }
----------------
int3 wrote:
> int3 wrote:
> > compnerd wrote:
> > > It seems that it would be better to replace `initProt` and `maxProt` with just `prot`?  Or just filling in the differences that require this would be a good idea.
> > Does it make sense to fill in code that we're not going to be able to test?
> I wrote it this way so it would be easy to "fill out" the missing bits when we support i386
I think that it does make sense to fill it out now even if we cannot test it.  Alternatively, we could defer the setup to the point where it is needed.  In the current state, it is very confusing and not very valuable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89420



More information about the llvm-commits mailing list