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 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.

cfe-commits mailing list

Reply via email to