[PATCH] D26882: Refactor how FunctionDecl handles constexpr:

[Hubert Tong via cfe-commits]

hubert.reinterpretcast added inline comments.


================
Comment at: include/clang/AST/Decl.h:1915
+    IsConstexprSpecified = IC;
+    IsConstexpr = IC;
+  }
----------------
How is the `inline` property transmitted here? Why does the `setImplicitlyConstexpr` function need to call `setImplicitlyInline`?


================
Comment at: include/clang/AST/Decl.h:1919
+  /// Flag that this function as implicitly constexpr
+  /// C++11 [dcl.constexpr]p2: constexpr functions and constexpr constructors
+  /// are implicitly inline functions (7.1.2).
----------------
The quote does not seem to be pertinent here. Maybe have it a few lines down?


================
Comment at: include/clang/AST/Decl.h:1923
+    IsConstexpr = IC;
+    setImplicitlyInline();
+  }
----------------
I am quite sure this is not the right thing to do when `IC` is `false`.


================
Comment at: lib/Sema/SemaDecl.cpp:8085
       // C++ Concepts TS [dcl.spec.concept]p2: Every concept definition is
       // implicity defined to be a constexpr declaration (implicitly inline)
+      NewFD->setImplicitlyConstexpr(true);
----------------
The parenthetical here seems to be no longer needed.


================
Comment at: lib/Sema/SemaDecl.cpp:9107
       << FixItHint::CreateRemoval(DS.getConstexprSpecLoc());
-    FD->setConstexpr(false);
+    FD->setImplicitlyConstexpr(false);
   }
----------------
This reads wrong to me. I get that the idea is that the function should not be semantically considered `constexpr`, but the choice of `setImplicitlyConstexpr` seems odd.


================
Comment at: test/CXX/concepts-ts/dcl.dcl/dcl.spec/dcl.spec.concept/p2.cpp:21
+static_assert(FCSizeOfU4<unsigned int>(), "");
+static_assert(FCSizeOfU4<long>(), "size of argument not equal to expected (4)"); // expected-error {{static_assert failed "size of argument not equal to expected (4)"}}
+
----------------
This does not strike me as being very portable (I may be mistaken about how `clang -cc1` works though). I think it would be much safer to use `char [8]` here (and in general for the size matching).



https://reviews.llvm.org/D26882