nwilson added inline comments.
================
Comment at: include/clang/AST/Decl.h:1915
+ IsConstexprSpecified = IC;
+ IsConstexpr = IC;
+ }
----------------
hubert.reinterpretcast wrote:
> How is the `inline` property transmitted here? Why does the
> `setImplicitlyConstexpr` function need to call `setImplicitlyInline`?
`inline` isn't //really// transmitted here. When we create a `FunctionDecl` in
`CreateNewFunctionDecl` we //could// use this rather than passing `isConstexpr`
as a parameter to the constructor. `setImplicitlyConstexpr` is intended to be
used downstream such as in `ActOnFunctionDeclarator` or
`CheckExplicitlyDefaultedSpecialMember`.
================
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).
----------------
hubert.reinterpretcast wrote:
> The quote does not seem to be pertinent here. Maybe have it a few lines down?
Sorry, can you point to where you're thinking below?
================
Comment at: include/clang/AST/Decl.h:1923
+ IsConstexpr = IC;
+ setImplicitlyInline();
+ }
----------------
hubert.reinterpretcast wrote:
> I am quite sure this is not the right thing to do when `IC` is `false`.
Good point. I //could// do `if (IC) setImplicitlyInline();`, but that doesn't
seem great with these smaller functions. Any suggestions by you or @rsmith
would be great!
================
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);
----------------
hubert.reinterpretcast wrote:
> The parenthetical here seems to be no longer needed.
I'll remove it.
================
Comment at: lib/Sema/SemaDecl.cpp:9107
<< FixItHint::CreateRemoval(DS.getConstexprSpecLoc());
- FD->setConstexpr(false);
+ FD->setImplicitlyConstexpr(false);
}
----------------
hubert.reinterpretcast wrote:
> 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.
Hmm, I'm not sure if we should keep around `setConstexpr` to handle cases
either... Do you or @rsmith have any opinion?
================
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)"}}
+
----------------
hubert.reinterpretcast wrote:
> 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).
>
Yeah, good point. I'll fix this.
https://reviews.llvm.org/D26882
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
