[PATCH] D12347: [MC/AsmParser] Avoid setting MCSymbol.IsUsed

[Vedant Kumar via llvm-commits]

vsk added a comment.

I tried the RAII helper object approach. It improves readability in some areas, but IMHO also leads to brittle code.

For one, some spots remain more readable with the `/*SetUsed*/` annotations. E.g, we would need to put the first conditional in its own scope to use the helper object here:

  else if (Sym->isUndefined(/*SetUsed*/ false) && !Sym->isUsed() && !Sym->isVariable()) { ... }
  else if (Sym->isVariable() && !Sym->isUsed() && allow_redef) { ... }

Secondly, you need to remember to create new helper objects every time a mutating accessor is invoked so that you don't accidentally grab an incorrect IsUsed state.

I'd prefer to stick with this revision. As a followup, I can work on (1) identifying the call sites which //intend// to mutate IsUsed, (2) forcing those call sites to be explicit with Sym->setUsed(), and (3) making the MCSymbol accessors read-only w.r.t IsUsed.

Let me know what you think!


http://reviews.llvm.org/D12347