thakis accepted this revision. thakis added a comment. This revision is now accepted and ready to land.
Good luck! I think the forward-looking statement in https://reviews.llvm.org/D43980 about this change here is a bit off, see below. The hasLocalStorage() comment below is probably the only interesting comment. ================ Comment at: clang/include/clang/Sema/Sema.h:1320 sema::FunctionScopeInfo *getCurFunction() const { - return FunctionScopes.back(); + return FunctionScopes.empty() ? nullptr : FunctionScopes.back(); } ---------------- The other patch description said "This means the FunctionScopes stack will often be empty, which will make getCurFunction() assert" -- did you change your mind, or did you mean "which will make the caller of getCurFunction() crash when it uses the null pointer"? ================ Comment at: clang/lib/Sema/AnalysisBasedWarnings.cpp:635 -/// CheckFallThroughForFunctionDef - Check that we don't fall off the end of a +/// CheckFallThroughForBody - Check that we don't fall off the end of a /// function that should return a value. Check that we don't fall off the end ---------------- nit: Just remove everything up to and including the `-` – we no longer repeat function names in comments. ================ Comment at: clang/lib/Sema/SemaDecl.cpp:11328 - if (var->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) + if (var->hasLocalStorage() && + var->getType().isDestructedType() == QualType::DK_nontrivial_c_struct) ---------------- the hasLocalStorage() check addition here seems unrelated? ================ Comment at: clang/lib/Sema/SemaExprCXX.cpp:1117 - const unsigned MaxFunctionScopesIndex = FunctionScopeIndexToStopAt ? - *FunctionScopeIndexToStopAt : FunctionScopes.size() - 1; + const int MaxFunctionScopesIndex = FunctionScopeIndexToStopAt + ? *FunctionScopeIndexToStopAt ---------------- Maybe add "// can be -1 if there's no current function scope and FunctionScopeIndexToStopAt is not set" since that seems pretty subtle. https://reviews.llvm.org/D44039 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits