[clang] [clang] Expose MC target options through -mllvm (PR #210087)

[Fangrui Song via cfe-commits]

MaskRay wrote:

I am in favor of this change, but please can you trim the comments? I don't think it's worth mentioning this new large-eh-encoding in the comments: it's probably one of the least used options in the wild.

```
● Yes — this has a fairly strong AI smell, mostly from verbosity + duplication rather than outright wrong content. Here's the breakdown.

  The tells

  1. Near-verbatim duplication of a 5-line block across files. The "register flags" comment and the "seed" comment are each copy-pasted between BackendUtil.cpp and cc1as_main.cpp with only token swaps (clang's -mllvm → -cc1as -mllvm, llc, llvm-mc → llvm-mc). The same explanation then appears a third time in the test-file header and a fourth time in the PR body. Four restatements of one idea is a classic generated-text
  signature.

  2. Comment-to-code ratio is inverted. The actual change is two lines per file (one static decl, one init). Each is wrapped in a 4–5 line comment — so ~4 comment lines per code line. LLVM convention here would be one line.

  3. Restating the obvious no-op case. Every seed comment ends with:

  ▎ // Absent any -mllvm flags this is equivalent to a default-constructed MCTargetOptions.

  That's filler — InitMCTargetOptionsFromFlags() with no flags obviously yields defaults. It reads like padding to "cover all cases."

  4. Explaining adjacent/other code.
  - "Clang's own driver-derived settings below overwrite whatever they cover" — narrates the lines that follow.
  - "whose accessors require the flags to have been registered" — narrates InitMCTargetOptionsFromFlags's internals.

  This is the "multi-line comment explaining what other code does" pattern you named.

  5. Test file re-explains what inline RUN comments already say. The 8-line header block spells out the precedence model, then each RUN group re-explains the same precedence for its specific case. The per-case comments are actually the useful ones; the header is redundant.

  What's legitimately warranted

  Not all of it is slop. A bare static llvm::mc::RegisterMCTargetOptionsFlags MCTargetOptionsFlags; genuinely looks like dead code, so one line saying "register MC flags so -mllvm can reach them (as llc/llvm-mc do)" is justified. And the precedence point (clang flags win over -mllvm) is the real non-obvious design decision worth stating — once.

  Tightened version

  // Register MC target-option flags (e.g. -large-eh-encoding) so they're
  // reachable via -mllvm, as llc/llvm-mc do.
  static llvm::mc::RegisterMCTargetOptionsFlags MCTargetOptionsFlags;
  // Seed from -mllvm first; clang's own flags below take precedence.
  Options.MCOptions = llvm::mc::InitMCTargetOptionsFromFlags();

  That preserves every fact a reader needs and drops ~15 lines of restatement. So: yes, strong smell — driven by duplication and over-explanation, fixable by collapsing each block to one line.

```

https://github.com/llvm/llvm-project/pull/210087