[PATCH] D100019: [lld-macho][nfc] Minor refactoring + clang-tidy fixes

Jez Ng via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Apr 7 10:20:35 PDT 2021


int3 added inline comments.


================
Comment at: lld/MachO/Driver.cpp:359-361
+// Parses LC_LINKER_OPTION contents, which can add additional command line
+// flags.
+void macho::parseLCLinkerOption(InputFile *f, unsigned argc, StringRef data) {
----------------
ahh well this was one of the formatting changes I committed yesterday, hence the tidy/format confusion 😅


================
Comment at: lld/MachO/Driver.cpp:1064-1067
+      std::vector<StringRef> extensions = {".tbd"};
+      for (const Arg *arg : args.filtered(OPT_sub_umbrella)) {
+        reexportHandler(arg, extensions);
+      }
----------------
oontvoo wrote:
> int3 wrote:
> > int3 wrote:
> > > 1. no need for braces
> > > 2. I'm pretty sure the compiler can hoist out the construction of the vector
> > > I'm pretty sure the compiler can hoist out the construction of the vector
> > 
> > well, I'm wrong: https://godbolt.org/z/szKbeojrd
> > > I'm pretty sure the compiler can hoist out the construction of the vector
> > well, I'm wrong: https://godbolt.org/z/szKbeojrd
> 
> Probably because it can't know what `foo` does with the reference - hoisting it out could change the logic entirely:
> 
> ```
> void foo(const std::vector<int>& v) {
>   static auto* last_vector = &v;
>   static bool first = true;
>   if (first) first = false;
>   else  assert (&v != last_vector);  
> }
> ```
hmm true. Though I'm not sure if temporary rvalues are guaranteed to be given a fresh address every time...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100019



More information about the llvm-commits mailing list