Changeset View
Standalone View
clang/lib/Sema/Sema.cpp
| Show First 20 Lines • Show All 443 Lines • ▼ Show 20 Lines | #include "clang/Basic/RISCVVTypes.def" | ||||
| } | } | ||||
| if (Context.getTargetInfo().hasBuiltinMSVaList()) { | if (Context.getTargetInfo().hasBuiltinMSVaList()) { | ||||
| DeclarationName MSVaList = &Context.Idents.get("__builtin_ms_va_list"); | DeclarationName MSVaList = &Context.Idents.get("__builtin_ms_va_list"); | ||||
| if (IdResolver.begin(MSVaList) == IdResolver.end()) | if (IdResolver.begin(MSVaList) == IdResolver.end()) | ||||
| PushOnScopeChains(Context.getBuiltinMSVaListDecl(), TUScope); | PushOnScopeChains(Context.getBuiltinMSVaListDecl(), TUScope); | ||||
| } | } | ||||
| if (auto *ND = dyn_cast_if_present<NamedDecl>(Context.getVaListTagDecl())) { | |||||
aaron.ballman: The API changed names recently. | |||||
| if (auto *DC = ND->getLexicalDeclContext(); DC->isStdNamespace()) { | |||||
| auto *NS = cast<NamespaceDecl>(DC); | |||||
| if (!StdNamespace) | |||||
| StdNamespace = NS; | |||||
| if (!Context.getTranslationUnitDecl()->containsDecl(NS)) | |||||
| PushOnScopeChains(NS, TUScope); | |||||
| if (!DC->containsDecl(ND)) { | |||||
| Scope S(TUScope, Scope::DeclScope, getDiagnostics()); | |||||
| PushDeclContext(&S, DC); | |||||
| PushOnScopeChains(ND, &S); | |||||
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. aaron.ballman: Is it valid to use a local variable as a `Scope` object? I would assume that scope goes away at… | |||||
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. mizvekov: I don't see anything problematic about allocating it on the stack like this, implementation… | |||||
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. erichkeane: I dont know much about "Scope", as it is particularly used in Parsing, but I DO see the comment… | |||||
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. aaron.ballman: The situation I'm worried about (and maybe it's not a valid concern) is that `Parser… | |||||
Yeah I checked that. The scope object is fairly innocuous, the EnterScope function uses a cache to stack them, which seems useful in performance intensive workloads, and in case the object needs to outlive the function that creates it. But here I see no problem just making it clear that this Scope is only used here, by simply allocating it on the stack. mizvekov: Yeah I checked that. The scope object is fairly innocuous, the `EnterScope` function uses a… | |||||
Great, thank you for double-checking, I appreciate it! aaron.ballman: Great, thank you for double-checking, I appreciate it! | |||||
| PopDeclContext(); | |||||
| } | |||||
| } else { | |||||
| assert(DC == Context.getTranslationUnitDecl()); | |||||
| if (!DC->containsDecl(ND)) | |||||
| PushOnScopeChains(ND, TUScope); | |||||
| } | |||||
| } | |||||
| DeclarationName BuiltinVaList = &Context.Idents.get("__builtin_va_list"); | DeclarationName BuiltinVaList = &Context.Idents.get("__builtin_va_list"); | ||||
| if (IdResolver.begin(BuiltinVaList) == IdResolver.end()) | if (IdResolver.begin(BuiltinVaList) == IdResolver.end()) | ||||
| PushOnScopeChains(Context.getBuiltinVaListDecl(), TUScope); | PushOnScopeChains(Context.getBuiltinVaListDecl(), TUScope); | ||||
| } | } | ||||
| Sema::~Sema() { | Sema::~Sema() { | ||||
| assert(InstantiatingSpecializations.empty() && | assert(InstantiatingSpecializations.empty() && | ||||
| "failed to clean up an InstantiatingTemplate?"); | "failed to clean up an InstantiatingTemplate?"); | ||||
| ▲ Show 20 Lines • Show All 2,246 Lines • Show Last 20 Lines | |||||
The API changed names recently.