[PATCH] D93263: [lld-macho] Implement option: -undefined TREATMENT

[Jez Ng via Phabricator via llvm-commits]

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

lgtm



================
Comment at: lld/MachO/SymbolTable.cpp:163
+  default:
+    fatal("malformed -undefined treatment");
+    return false;
----------------
gkm wrote:
> gkm wrote:
> > int3 wrote:
> > > with the change suggested above, this could be `llvm_unreachable` instead of `fatal`
> > I dropped `default:` entirely, because clang warns that it is unused as every enum case is explicitly handled.
> I thought I could drop `unknown` from the enum, but that wasn't workable for giving a diagnostic on `StringSwitch<UndefinedSymbolTreatment>(...).Default(...)`, so `default: llvm_unreachable()` is necessary after all.
you could also have `case UndefinedSymbolTreatment::unknown:` instead of `default:` (while retaining the `llvm_unreachable`) -- that way we still benefit from Clang's exhaustiveness checks


================
Comment at: lld/MachO/SymbolTable.cpp:164
+  default:
+    llvm_unreachable("unkown undefined-symbol treatment");
+  }
----------------
s/unkown/unknown/


================
Comment at: lld/test/MachO/invalid/undefined-symbol.s:13
 # RUN:     FileCheck %s -DSYM=_bar -DFILENAME='foo.a(foo.o)'
-# CHECK: error: undefined symbol [[SYM]], referenced from [[FILENAME]]
+# CHECK: error: undefined symbol '[[SYM]]', referenced from [[FILENAME]]
 
----------------
fwiw it appears that the ELF and COFF ports use the following format:

```
error: undefined symbol: $SYM
>>> referenced by $FILE
```

but we can always tweak our format later on


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93263