This revision was automatically updated to reflect the committed changes.
Closed by commit rL260074: [Concepts] Implement a portion of Concepts
TS[dcl.spec.concept]p1 by (authored by nwilson).
Changed prior to commit:
http://reviews.llvm.org/D13357?vs=46863=47160#toc
Repository:
rL LLVM
rsmith accepted this revision.
rsmith added a comment.
This revision is now accepted and ready to land.
LGTM
http://reviews.llvm.org/D13357
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
hubert.reinterpretcast added a comment.
Minor comments; otherwise, LGTM.
Comment at: lib/Sema/SemaDecl.cpp:6007
@@ +6006,3 @@
+ // applied only to the definition of a [...] variable template, declared
+ // in namespace scope. [...] A concept definition refers to [...]
nwilson updated this revision to Diff 46854.
nwilson added a comment.
- This update removes the parameter in `TemplateDecl::setConcept` since there
isn't a need to mark a concept false.
http://reviews.llvm.org/D13357
Files:
include/clang/AST/DeclTemplate.h
nwilson updated this revision to Diff 46863.
nwilson added a comment.
- Address Hubert's comments about the quoted section of the TS.
http://reviews.llvm.org/D13357
Files:
include/clang/AST/DeclTemplate.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
nwilson updated this revision to Diff 46588.
nwilson added a comment.
- Fix a couple of comments to reflect the Patch.
- Clang-format the changes in this Patch.
http://reviews.llvm.org/D13357
Files:
include/clang/AST/DeclTemplate.h
include/clang/Basic/DiagnosticSemaKinds.td
rsmith added inline comments.
Comment at: include/clang/AST/DeclTemplate.h:374
@@ +373,3 @@
+ bool isConcept() const { return TemplatedDecl.getInt(); }
+ void setConcept(bool IC) { TemplatedDecl.setInt(true); }
+
I would prefer to not have a setter at all, but
nwilson marked 5 inline comments as done.
nwilson added a comment.
Marking some comments Done which were fixed in previous updates.
http://reviews.llvm.org/D13357
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
nwilson added inline comments.
Comment at: include/clang/AST/DeclTemplate.h:374
@@ +373,3 @@
+ bool isConcept() const { return TemplatedDecl.getInt(); }
+ void setConcept(bool IC) { TemplatedDecl.setInt(true); }
+
rsmith wrote:
> I would prefer to not have a
nwilson added a comment.
Ping.
@rsmith - would you also mind clarifying the comment regarding `setConcept(bool
IC)` at to whether it should exist at all or simply not have any params?
http://reviews.llvm.org/D13357
___
cfe-commits mailing list
hubert.reinterpretcast added inline comments.
Comment at: include/clang/AST/DeclTemplate.h:375
@@ +374,3 @@
+ bool isConcept() const { return TemplatedDecl.getInt(); }
+ void setConcept(bool IC) { TemplatedDecl.setInt(true); }
+
nwilson wrote:
>
nwilson added inline comments.
Comment at: include/clang/AST/DeclTemplate.h:375
@@ +374,3 @@
+ bool isConcept() const { return TemplatedDecl.getInt(); }
+ void setConcept(bool IC) { TemplatedDecl.setInt(true); }
+
hubert.reinterpretcast wrote:
> The parameter
nwilson added inline comments.
Comment at: include/clang/AST/DeclTemplate.h:377
@@ -376,3 +376,3 @@
NamedDecl *TemplatedDecl;
TemplateParameterList* TemplateParams;
I can't seem to follow the link properly, but I'm assuming it's supposed to be
where
nwilson added a comment.
Ping. Now that the holidays are over-ish, as Aaron said in one of his Patches.
http://reviews.llvm.org/D13357
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
nwilson added inline comments.
Comment at: include/clang/AST/DeclTemplate.h:836
@@ -835,2 +835,3 @@
+ bool IsConcept : 1;
protected:
rsmith wrote:
> This might make more sense on `TemplateDecl`, since we also want this flag
> for `VarTemplateDecl`s. In any
nwilson updated this revision to Diff 43670.
nwilson added a comment.
- Store the IsConcept boolean flag in TemplateDecl by making TemplatedDecl an
IntPointerPair, and move the associated member functions into TemplateDecl.
- Remove unnecessary quoted comment.
- Remove an extra space where the
nwilson updated this revision to Diff 43469.
nwilson added a comment.
Updating to r256247
http://reviews.llvm.org/D13357
Files:
include/clang/AST/DeclTemplate.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplate.cpp
rsmith added inline comments.
Comment at: include/clang/AST/DeclTemplate.h:836
@@ -835,2 +835,3 @@
+ bool IsConcept : 1;
protected:
This might make more sense on `TemplateDecl`, since we also want this flag for
`VarTemplateDecl`s. In any case, please use
faisalv added a subscriber: Nate.
Comment at: include/clang/AST/DeclTemplate.h:836
@@ -835,2 +835,3 @@
+ bool IsConcept : 1;
protected:
rsmith wrote:
> This might make more sense on `TemplateDecl`, since we also want this flag
> for `VarTemplateDecl`s. In
nwilson added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:7659
@@ +7658,3 @@
+Diag(D.getDeclSpec().getConceptSpecLoc(),
+ diag::err_concept_specified_specialization) << 1;
+ }
rsmith wrote:
> hubert.reinterpretcast wrote:
> >
nwilson updated this revision to Diff 41441.
nwilson added a comment.
Updating to r254337
http://reviews.llvm.org/D13357
Files:
include/clang/AST/Decl.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplate.cpp
rsmith added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:7659
@@ +7658,3 @@
+Diag(D.getDeclSpec().getConceptSpecLoc(),
+ diag::err_concept_specified_specialization) << 1;
+ }
hubert.reinterpretcast wrote:
> nwilson wrote:
> >
hubert.reinterpretcast added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:7659
@@ +7658,3 @@
+Diag(D.getDeclSpec().getConceptSpecLoc(),
+ diag::err_concept_specified_specialization) << 1;
+ }
nwilson wrote:
> hubert.reinterpretcast
nwilson added inline comments.
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2003
@@ +2002,3 @@
+def err_concept_specified_specialization : Error<
+ "%'concept' cannot be applied on an "
+ "%select{explicit instantiation|explicit specialization|partial
aaron.ballman added a comment.
Aside from one question, LGTM.
~Aaron
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:2003
@@ +2002,3 @@
+def err_concept_specified_specialization : Error<
+ "%'concept' cannot be applied on an "
+ "%select{explicit
nwilson updated this revision to Diff 40105.
nwilson added a comment.
- Remove marking a variable concept invalid when specialized since we'll only
look at the primary template downstream. This removal let's us use the same
recovery path as before when 'concept' is specified on a non-template.
nwilson updated this revision to Diff 39030.
nwilson added a comment.
Updating to r251898
http://reviews.llvm.org/D13357
Files:
include/clang/AST/Decl.h
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
lib/Sema/SemaTemplate.cpp
nwilson added inline comments.
Comment at: lib/Sema/SemaDecl.cpp:5909
@@ +5908,3 @@
+ diag::err_concept_specified_specialization)
+ << (IsPartialSpecialization ? 2 : 1);
+NewVD->setInvalidDecl(true);
Hmm, I'd lean toward leaving it as
nwilson added inline comments.
Comment at: include/clang/AST/Decl.h:1577
@@ -1576,2 +1576,3 @@
bool IsConstexpr : 1;
+ bool IsConcept : 1;
faisalv wrote:
> My inclination would have been to add this bit to FunctionTemplateDecl,
> instead of to every
faisalv added inline comments.
Comment at: include/clang/AST/Decl.h:1577
@@ -1576,2 +1576,3 @@
bool IsConstexpr : 1;
+ bool IsConcept : 1;
My inclination would have been to add this bit to FunctionTemplateDecl, instead
of to every FunctionDecl - since not
nwilson updated this revision to Diff 37529.
nwilson added a comment.
Addressing Richard's other comment regarding the FunctionDecl being declared a
concept check. Also, remove setting the FunctionDecl being Invalid in the
Specialization check since the downstream parts of Clang should look at
nwilson marked 2 inline comments as done.
Comment at: lib/Sema/SemaDecl.cpp:7886
@@ -7863,1 +7885,3 @@
+
+ if (NewFD->isInvalidDecl() && !NewFD->isConcept()) {
HasExplicitTemplateArgs = false;
Maybe there could be a problem further down if we think
32 matches
Mail list logo
