[PATCH] D66361: Improve behavior in the case of stack exhaustion.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM aside from an accidental newline change.



================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:15
 let CategoryName = "Semantic Issue" in {
-
 def note_previous_decl : Note<"%0 declared here">;
----------------
Spurious removal?


================
Comment at: lib/Sema/SemaExpr.cpp:15070-15079
+  // Trivial default constructors and destructors are never actually used.
+  // FIXME: What about other special members?
+  if (Func->isTrivial() && !Func->hasAttr<DLLExportAttr>() &&
+      OdrUse == OdrUseContext::Used) {
+    if (auto *Constructor = dyn_cast<CXXConstructorDecl>(Func))
+      if (Constructor->isDefaultConstructor())
+        OdrUse = OdrUseContext::FormallyOdrUsed;
----------------
rsmith wrote:
> aaron.ballman wrote:
> > This seems unrelated to the patch?
> I agree it seems that way, but it is related:
> 
> The block of code below that got turned into a lambda contains early exits via `return` for the cases that get downgraded to `FormallyOdrUsed` here. We used to bail out of the whole function and not mark these trivial special member functions as "used" (in the code after the `NeedDefinition && !Func->getBody()` condition), which seems somewhat reasonable since we don't actually need definitions of them; other parts of Clang (specifically the static analyzer) have developed a reliance on that behavior.
> 
> So this is preserving the existing behavior and (hopefully) making it more obvious what that behavior is, rather than getting that behavior by an early exit from the "need a definition?" portion of this function leaking into the semantics of the "mark used" portion.
> 
> I can split this out and make this change first if you like.
> I can split this out and make this change first if you like.

No need, I appreciate the explanation!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66361/new/

https://reviews.llvm.org/D66361