[PATCH] D65752: [Sema] Refactor LookupVisibleDecls. NFC
Ilya Biryukov via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
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?
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
More information about the cfe-commits