[PATCH] D50494: [LLD] Reserve memory in implicitly set LMARegions

[Konstantin Schwarz via Phabricator via llvm-commits]

kschwarz added inline comments.


================
Comment at: ELF/LinkerScript.cpp:723-724
+static bool areFlagsCompatible(MemoryRegion *M, OutputSection *Sec) {
+  return (M && (!M->Flags || (M->Flags & Sec->Flags)) &&
+          ((M->NegFlags & Sec->Flags) == 0));
+}
----------------
ruiu wrote:
> Remove extraneous parentheses. I.e.
> 
>   return M && (!M->Flags || (M->Flags & Sec->Flags)) &&
>           (M->NegFlags & Sec->Flags) == 0;
> 
> But perhaps it is better to deconstruct it a bit by using more lines:
> 
>   if (!M)
>     return false;
>   if (M->Flags && !(M->Flags & Sec->Flags)
>     return false;
>   return (M->NegFlags & Sec->Flags) == 0;
> 
> Now, my question is, can really `M` be a nullptr?
Sure, I can simplify it a bit. 

Currently this function can be called with M being a nullptr (from `findLMARegion`). We can add a check for nullptr there and eliminate the check in `areFlagsCompatible`, which seems more reasonable. 


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D50494