[PATCH] D44039: [Sema] Make getCurFunction() return null outside function parsing

[Nico Weber via Phabricator via cfe-commits]

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