aaron.ballman added inline comments.
================ Comment at: clang/lib/Sema/Sema.cpp:460-462 + Scope S(TUScope, Scope::DeclScope, getDiagnostics()); + PushDeclContext(&S, DC); + PushOnScopeChains(ND, &S); ---------------- erichkeane wrote: > mizvekov wrote: > > aaron.ballman wrote: > > > Is it valid to use a local variable as a `Scope` object? I would assume > > > that scope goes away at the end of this compound statement? I didn't spot > > > other cases where we did this -- usually we call `Parser::EnterScope()` > > > which allocates a new `Scope` that `Sema` then uses. > > I don't see anything problematic about allocating it on the stack like > > this, implementation-wise. > > > > I thought it would be a bit counter-intuitive to use parser functions here, > > since these declarations are entirely synthetic. > I dont know much about "Scope", as it is particularly used in Parsing, but I > DO see the comment says: "A scope is a transient data structure that is used > while parsing the program. It assists with resolving identifiers to the > appropriate declaration." > > Which means to me it doesn't need to out-live its parsing time. I THINK it > gets replaced with a CXXScope at one point, but I dont know when/where that > happens. > The situation I'm worried about (and maybe it's not a valid concern) is that `Parser::EnterScope` uses a cache of scopes that this bypasses and I'm not certain whether that's a good thing or not. I *think* it might be reasonable (`Scope` is just a holder for a `DeclContext` and that's the object which has all the declarations added to and removed from, and that context outlives the `Scope` object), but it's worth double-checking. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136886/new/ https://reviews.llvm.org/D136886 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
