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 optional nested name specifier used when naming the concept.
----------------
remove `protected` (and `private:` below), we don't inherit from this
================
Comment at: clang/include/clang/AST/ExprConcepts.h:46
public:
using SubstitutionDiagnostic = std::pair<SourceLocation, std::string>;
----------------
while here: this is unused, remove?
================
Comment at: clang/include/clang/AST/ExprConcepts.h:48
protected:
+ ConceptReference *ConceptRef;
----------------
while here: this class is final, so these can be private instead of protected
================
Comment at: clang/lib/AST/TypeLoc.cpp:625
-DeclarationNameInfo AutoTypeLoc::getConceptNameInfo() const {
- return DeclarationNameInfo(getNamedConcept()->getDeclName(),
- getLocalData()->ConceptNameLoc);
+static ConceptReference *createConceptReference(ASTContext &Context,
+ SourceLocation Loc,
----------------
nit: createTrivialConceptReference?
hinting at the relationship to trivial TypeSourceInfo
maybe a comment too?
```
// Builds a ConceptReference where all locations point at the same token,
// for use in trivial TypeSourceInfo for constrained AutoType
```
================
Comment at: clang/unittests/AST/SourceLocationTest.cpp:1025
+)cpp";
+ // FIXME: expected range should be (2, 1, 2, 3)
+ Verifier.expectRange(2, 1, 2, 1);
----------------
why that range?
SourceRanges are generally "closed token ranges", so the endpoint is the
*beginning* of the last token *in* the range.
`CCC` is (2, 1, 2, 1)
================
Comment at: clang/unittests/AST/SourceLocationTest.cpp:1073
+)cpp";
+ // FIXME: expected range should be (2, 11, 2, 13)
+ Verifier.expectRange(2, 11, 2, 11);
----------------
as above
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D155858/new/
https://reviews.llvm.org/D155858
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
