This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc2bf9baf5987: Add a concept AST node. (authored by massberg).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm
massberg updated this revision to Diff 554743.
massberg marked 7 inline comments as done.
massberg added a comment.
Fixed remaining nits. Thanks for the reviews and comments everyone!
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155858/new/
https:
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
LGTM, just nits!
The SourceRanges are actually correct :-)
Comment at: clang/include/clang/AST/ASTConcept.h:124
class ConceptReference {
protected:
// \brief The
massberg added inline comments.
Comment at: clang/lib/Sema/TreeTransform.h:6824-6836
+ unsigned size = TL.getTypePtr()->getTypeConstraintArguments().size();
+ TemplateArgumentLocInfo *TALI = new TemplateArgumentLocInfo[size];
+ TemplateSpecializationTypeLoc::initia
massberg updated this revision to Diff 554660.
massberg marked 3 inline comments as done.
massberg added a comment.
Resolve remaining comments.
- Added tests for new location functions of `ConceptReference`. There is an
existing bug with the end location if there are no template arguments. As it
sammccall added a comment.
OK, so I learned a bit about initializeLocal/trivial TypeSourceInfo. Mostly
things I didn't want to know :-)
TL;DR I think the current version of the patch is right.
The crash is caused by:
1. `CheckTemplateArgument` calls `SubstType` without passing a `TypeLoc`, whic
massberg updated this revision to Diff 554257.
massberg marked 7 inline comments as done.
massberg added a comment.
Resolve more of the open comments.
- add `getBeginLoc()`, `getEndLoc()` and `getSourceRange()` to
`ConceptReference` and updated corresponding functions in
`ConceptSpecializationE
sammccall added inline comments.
Comment at: clang/lib/AST/TypeLoc.cpp:625
-DeclarationNameInfo AutoTypeLoc::getConceptNameInfo() const {
- return DeclarationNameInfo(getNamedConcept()->getDeclName(),
- getLocalData()->ConceptNameLoc);
+ConceptRefer
massberg updated this revision to Diff 552742.
massberg marked 4 inline comments as done.
massberg added a comment.
Resolved the next round of comments. There are still some left to be addressed.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155858/
erichkeane added a comment.
So I did a quick run through, I think this is a fine idea and an improvement.
However, @sammccall has given some very good feedback that I'd like to have him
do the final approval.
Comment at: clang/include/clang/AST/ASTConcept.h:213
Expr *Imme
erichkeane added a comment.
Hi! I was away on vacation, so I didn't get a chance to take a look at this.
I'm currently digging my way through my 'back to work' tasks, but will keep
this on my list of things to review ASAP.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
ht
sammccall added inline comments.
Comment at: clang/lib/Sema/TreeTransform.h:6824-6836
+ unsigned size = TL.getTypePtr()->getTypeConstraintArguments().size();
+ TemplateArgumentLocInfo *TALI = new TemplateArgumentLocInfo[size];
+ TemplateSpecializationTypeLoc::initi
massberg marked 2 inline comments as done.
massberg added inline comments.
Comment at: clang/lib/Sema/TreeTransform.h:6824-6836
+ unsigned size = TL.getTypePtr()->getTypeConstraintArguments().size();
+ TemplateArgumentLocInfo *TALI = new TemplateArgumentLocInfo[size];
+
massberg marked 13 inline comments as done.
massberg added a comment.
I have resolved some of the comments, but I need another round to finish all of
the, Thanks to all reviewers so far!
Comment at: clang/include/clang/AST/ExprConcepts.h:90
+ // NOTE(massberg): For the first
massberg updated this revision to Diff 552307.
massberg added a comment.
Resolve some of the reviewer's comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155858/new/
https://reviews.llvm.org/D155858
Files:
clang/include/clang/AST/ASTConcep
sammccall added a comment.
(There are still outstanding comments e.g. in ASTImport)
I think it would be useful to add to the patch description:
- the current deficiencies of ConceptReference that make it not a well-behaved
AST node now
- which of these are addressed in this patch (RecursiveASTV
massberg updated this revision to Diff 549066.
massberg marked an inline comment as done.
massberg added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: jplehr, sstefan1.
Only call `AddConceptReference` if there is a concept reference.
Repository:
rG LLVM Github Monore
cor3ntin added inline comments.
Comment at: clang/include/clang/AST/ExprConcepts.h:140-143
SourceLocation getBeginLoc() const LLVM_READONLY {
-if (auto QualifierLoc = getNestedNameSpecifierLoc())
+if (auto QualifierLoc = CR->getNestedNameSpecifierLoc())
return Q
hokein added a subscriber: aaron.ballman.
hokein added a comment.
adding @aaron.ballman, who might have opinion on this.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155858/new/
https://reviews.llvm.org/D155858
___
massberg marked an inline comment as done.
massberg added inline comments.
Comment at: clang/include/clang/AST/ASTConcept.h:118
/// template arguments.
-class ConceptReference {
+class ConceptLoc {
protected:
cor3ntin wrote:
> hokein wrote:
> > I'm not sure the
massberg updated this revision to Diff 547804.
massberg edited the summary of this revision.
massberg added a comment.
Rename `ConceptLoc` back to `ConceptReference`.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155858/new/
https://reviews.llvm.or
cor3ntin added inline comments.
Comment at: clang/include/clang/AST/ASTConcept.h:118
/// template arguments.
-class ConceptReference {
+class ConceptLoc {
protected:
hokein wrote:
> I'm not sure the `ConceptLoc` is a reasonable name for this structure.
> 1) In
massberg created this revision.
massberg added a reviewer: sammccall.
Herald added subscribers: ChuanqiXu, martong.
Herald added a reviewer: shafik.
Herald added a project: All.
massberg updated this revision to Diff 544722.
massberg added a comment.
massberg updated this revision to Diff 545601.
m
23 matches
Mail list logo
