leonardchan marked 2 inline comments as done. leonardchan added inline comments.
================ Comment at: lib/Parse/ParseStmt.cpp:102-104 + Actions.PushExpressionEvaluationContext( + Actions.ExprEvalContexts.back().Context); ParenBraceBracketBalancer BalancerRAIIObj(*this); ---------------- rsmith wrote: > Hmm, we call `ParseStatementOrDeclaration` rather a lot. Can we pull this up > into its ultimate callers instead: > > * `Parser::ParseFunctionDefinition` > * `Parser::ParseLateTemplatedFuncDef` > * `Parser::ParseLexedMethodDef` > * `Parser::ParseLexedObjCMethodDefs` > > ? Actually, we can do better than that: those functions all call either > `ActOnStartOfFunctionDef` or `ActOnStartOfObjCMethodDef` first, and > `ActOnFinishFunctionBody` when they're done. We should put the > `PushExpressionEvaluationContext` and `PopExpressionEvaluationContext` calls > in those functions instead. > > We're missing at least one other place: when parsing the initializer for a > global variable, in C++ we call `Sema::ActOnCXXEnterDeclInitializer`, which > pushes a potentially-evaluated context. But in C, the `InitializerScopeRAII` > object (in `Parser::ParseDeclaratoinAfterDeclaratorAndAttributes`) doesn't > call into `Sema` and we don't ever push a suitable expression evaluation > context. > > You can find if any other places are missing by removing `Sema`'s global > `ExpressionEvaluationContext` and adding an assert that fires if we try to > parse an expression with no `ExpressionEvaluationContext`, and then running > the unit tests. (And we should check in that change to ensure this doesn't > regress.) @rsmith Would it be simpler to instead push and pop at the start and end of `Parser::ParseExternalDeclaration`? I'm currently working on your suggestion of removing the Sema global `ExpressionEvaluationContext` and find that a lot of accesses to the `ExprEvalContexts` stack stem from this method. `ActOnStartOfFunctionDef` and `ActOnFinishFunctionBody` still work on anything in a function body, but I keep running into many cases for declarations declared globally that could be easily caught if instead I push and pop contexts at the start and end of `Parser::ParseExternalDeclaration`. Do you think this is a good idea or is there something that I may be missing from pushing and popping here? This still accomplishes the goal of not reusing the global Sema context and I will still be able to check for `noderef` on every push and pop this way. Repository: rC Clang https://reviews.llvm.org/D49511 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
