[NFC] Add some assertions to placate my paranoia about sharing a variant bit across FunctionDecl and CXXDeductionGuideDecl - should I do this?

[faisalv]

Summary

I'd like to harden my patch here: https://reviews.llvm.org/rL316292 by adding some assertions.

But since the assertions in Decl,.h (FunctionDecl) require knowledge from DeclCXX.h (CXXDeductionGuideDecl),- my question is: In order to keep the member functions inline I factored them out into a separate header file that I included in certain areas.

Is this an acceptable pattern?

Or does anyone have any other preferred engineering suggestions?

Thanks!

[lichray]

Isn't it already an UB if someone set WillHaveBody and but later IsCopyDeductionCandidate being read, vice versa?

[faisalv]

In D39166#903131, @lichray wrote:

Isn't it already an UB if someone set WillHaveBody and but later IsCopyDeductionCandidate being read, vice versa?

Yes that would be UB - but I'm not sure I see how that would happen w the current implementation. Hence I thought I'd add the assertions to be certain.

[aaron.ballman]

I'll let Richard comment on whether this pattern is reasonable or not, but I have some very minor nits in the meantime.

include/clang/AST/Decl.h

1683

Since -> Because

(Might as well fix the other one as well while you're at it.)

include/clang/AST/InlineDeclMembers.h

10

This comment seems incorrect.

20

Spurious newline.

Because this functionality will always be for inline Decl members, I think it would make more sense to put the declarations into namespace clang rather than use a qualified name.

34

Another spurious newline.

35

Whitespace is incorrect here.

[faisalv]

Incorporated Aaron's feedback (although not sure if I caugh tall the white space issues).

Additionally, I was thinking of reordering the bits - and using UseSEHTry (this bit really makes no sense and even the chance of someone triggering its use erroneously (which can happen with the current bit if someone gives the deduction guide a body, should be zero) as my fusion bit.

I'm also not opposed to just adding a bit to the end of the FunctionDecl bit sequence.

Thanks for taking a look at the patch Aaron!

[faisalv]

Just added an additional bit-field to FunctionDecl in https://reviews.llvm.org/rL317984

include/clang/AST/InlineDeclMembers.h

35

not sure I follow?

include/clang/AST/Decl.h

 
 protected:
   // Since a Deduction Guide [C++17] will never have a body, we can share the
-  // storage, and use a different name.
+  // storage, and use a different name.  Since WillHaveBody is not serialized we
+  // don't need to worry about collisions there.
+
   union {
     /// Indicates if the function declaration will have a body, once we're done
     /// parsing it.

   void setHasSkippedBody(bool Skipped = true) { HasSkippedBody = Skipped; }
 
   /// True if this function will eventually have a body, once it's fully parsed.
-  bool willHaveBody() const { return WillHaveBody; }
-  void setWillHaveBody(bool V = true) { WillHaveBody = V; }
+  bool willHaveBody() const;
+  void setWillHaveBody(bool V = true);
 
   void setPreviousDeclaration(FunctionDecl * PrevDecl);
 

include/clang/AST/InlineDeclMembers.h

+//===- InlineDeclMembers.h - Decl.h Members that must be inlined -*- C++ -*-==//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+//  This file defines the Decl subclasses.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_AST_INLINEDECLMEMBERS_H
+#define LLVM_CLANG_AST_INLINEDECLMEMBERS_H
+
+#include "clang/AST/Decl.h"
+#include "clang/AST/DeclCXX.h"
+
+
+inline bool clang::FunctionDecl::willHaveBody() const {
+  assert(!isa<CXXDeductionGuideDecl>(this) &&
+    "must not be called on a deduction guide since we share this data "
+    "member while giving it different semantics");
+  return WillHaveBody;
+}
+inline void clang::FunctionDecl::setWillHaveBody(bool V) {
+  assert(!isa<CXXDeductionGuideDecl>(this) &&
+    "must not be called on a deduction guide since we share this data "
+    "member while giving it different semantics");
+  WillHaveBody = V;
+}
+
+
+#endif  //LLVM_CLANG_AST_INLINEDECLMEMBERS_H
+
+

include/clang/Sema/Sema.h

 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprObjC.h"
 #include "clang/AST/ExternalASTSource.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/AST/LocInfoType.h"
 #include "clang/AST/MangleNumberingContext.h"
 #include "clang/AST/NSAPI.h"

lib/AST/Decl.cpp

 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/AST/PrettyPrinter.h"
 #include "clang/AST/Stmt.h"
 #include "clang/AST/TypeLoc.h"

lib/AST/DeclCXX.cpp

 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/Expr.h"
 #include "clang/AST/ExprCXX.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/AST/ODRHash.h"
 #include "clang/AST/TypeLoc.h"
 #include "clang/Basic/IdentifierTable.h"

lib/Parse/ParseCXXInlineMethods.cpp

 
 #include "clang/Parse/Parser.h"
 #include "clang/AST/DeclTemplate.h"
+#include "clang/AST/InlineDeclMembers.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"