[PATCH] D56722: [lld] [ELF] Support for warn-once option

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added a comment.

I think I'm fine with this feature. I wouldn't personally use this that much, but I can imagine that some people would find this useful.



================
Comment at: ELF/Config.h:183
   bool WarnMissingEntry;
+  bool WarnOnce = false;
   bool WarnSymbolOrdering;
----------------
Remove `= false`


================
Comment at: ELF/Driver.cpp:867
       Args.hasFlag(OPT_warn_ifunc_textrel, OPT_no_warn_ifunc_textrel, false);
   Config->WarnSymbolOrdering =
       Args.hasFlag(OPT_warn_symbol_ordering, OPT_no_warn_symbol_ordering, true);
----------------
This is the right place to set to `Config->WarnOnce`


================
Comment at: ELF/Driver.cpp:1110-1112
+    case OPT_warn_once:
+      Config->WarnOnce = true;
+      break;
----------------
instead of this place.


================
Comment at: ELF/Options.td:376
+def warn_once: F<"warn-once">,
+  HelpText<"Warn only once per undefined symbol">;
 
----------------
per -> for each


================
Comment at: ELF/Symbols.h:214
+  // True if this symbol is already reported as undefined
+  unsigned IsWarned : 1;
+
----------------
This comment should end with a full stop. Please also mention that this bit is used for `--warn-once`.

IsWarned -> Warned


================
Comment at: test/ELF/warn-once.s:19
+  call foo;
\ No newline at end of file

----------------
Please make sure that this file ends with a newline character.


Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D56722