[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC

Mon Aug 12 02:53:34 PDT 2019

ilya-biryukov added a subscriber: doug.gregor.
ilya-biryukov added a comment.

In D65752#1623914 <https://reviews.llvm.org/D65752#1623914>, @sammccall wrote:

> This looks reasonable to me, there are a couple of variations you might think about:
>
> - also treat QualifiedNameLookup as an option, and override in places with an RAII pattern like `ScopedOverride Unqual(QualifiedNameLookup, false)` (why don't we have a class like that?). This would further reduce args.

I actually think it's good to have it as an explicit option. Qualified and unqualified lookup are so vastly different use-cases, that having the flag that discriminates them as a function parameter looks like a better trade-off.
Besides, RAII objects are more code and more complicated.

> - put the options in a `LookupOpts` struct and pass it by const reference - this is a less invasive change

Also an option. I personally like the current version more even though it's more invasive; it adds more structure IMO.

Those decisions are personal preferences, I don't have strong opinions there.
@doug.gregor as an original author of the code, do you have any opinions on where this code should go?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65752