rsmith added inline comments.
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1977
@@ -1976,1 +1976,3 @@
+def err_function_concept_exception_spec : Error<
+ "function concept can not have exception specifiers">;
----------------
def err -> def err
can not -> cannot
specifiers -> specification
================
Comment at: include/clang/Sema/DeclSpec.h:1330-1336
@@ -1325,5 +1329,9 @@
- SourceLocation getExceptionSpecLoc() const {
- return SourceLocation::getFromRawEncoding(ExceptionSpecLoc);
+ SourceLocation getExceptionSpecLocBeg() const {
+ return SourceLocation::getFromRawEncoding(ExceptionSpecLocBeg);
+ }
+
+ SourceLocation getExceptionSpecLocEnd() const {
+ return SourceLocation::getFromRawEncoding(ExceptionSpecLocEnd);
}
----------------
Please add a `getExceptionSpecRange` function returning the `SourceRange`...
================
Comment at: lib/Sema/SemaDecl.cpp:7447-7450
@@ -7446,1 +7446,6 @@
+ if (const FunctionProtoType *FPT = R->getAs<FunctionProtoType>()) {
+ if (FPT->hasExceptionSpec()) {
+ auto LocBeg = D.getFunctionTypeInfo().getExceptionSpecLocBeg();
+ auto LocEnd = D.getFunctionTypeInfo().getExceptionSpecLocEnd();
+ Diag(LocBeg, diag::err_function_concept_exception_spec)
----------------
This will assert if there isn't a `FunctionTypeInfo` for the declaration, which
can theoretically happen if it's declared via an (ill-formed today) `typedef`.
(It also might not provide a source range if the exception specification is
implicit, for instance because the function template is a destructor or
deallocation function, but passing an empty SourceRange to the FixItHint should
just result it in being ignored.)
================
Comment at: lib/Sema/SemaDecl.cpp:7449-7452
@@ +7448,6 @@
+ if (FPT->hasExceptionSpec()) {
+ auto LocBeg = D.getFunctionTypeInfo().getExceptionSpecLocBeg();
+ auto LocEnd = D.getFunctionTypeInfo().getExceptionSpecLocEnd();
+ Diag(LocBeg, diag::err_function_concept_exception_spec)
+ << FixItHint::CreateRemoval(SourceRange(LocBeg, LocEnd));
+ NewFD->setInvalidDecl();
----------------
... and use it here.
================
Comment at: lib/Sema/SemaDecl.cpp:7454-7455
@@ +7453,4 @@
+ NewFD->setInvalidDecl();
+ }
+ else {
+ NewFD->setType(Context.getFunctionType(
----------------
No newline before `else`.
================
Comment at: lib/Sema/SemaDecl.cpp:7456
@@ +7455,3 @@
+ else {
+ NewFD->setType(Context.getFunctionType(
+ FPT->getReturnType(), FPT->getParamTypes(),
----------------
rsmith wrote:
> Use `ASTContext::adjustExceptionSpec(NewFD, EST_BasicNoexcept)` for this.
Add a comment here indicating why we're doing this. Maybe a snippet of the
relevant rule from the TS?
================
Comment at: lib/Sema/SemaDecl.cpp:7456-7458
@@ +7455,5 @@
+ else {
+ NewFD->setType(Context.getFunctionType(
+ FPT->getReturnType(), FPT->getParamTypes(),
+ FPT->getExtProtoInfo().withExceptionSpec(EST_BasicNoexcept)));
+ }
----------------
Use `ASTContext::adjustExceptionSpec(NewFD, EST_BasicNoexcept)` for this.
================
Comment at: lib/Sema/SemaDecl.cpp:7456-7458
@@ +7455,5 @@
+ else {
+ NewFD->setType(Context.getFunctionType(
+ FPT->getReturnType(), FPT->getParamTypes(),
+ FPT->getExtProtoInfo().withExceptionSpec(EST_BasicNoexcept)));
+ }
----------------
rsmith wrote:
> rsmith wrote:
> > Use `ASTContext::adjustExceptionSpec(NewFD, EST_BasicNoexcept)` for this.
> Add a comment here indicating why we're doing this. Maybe a snippet of the
> relevant rule from the TS?
You don't seem to have test coverage for this part. Maybe test it with
`static_assert(noexcept(SomeConcept<T>()));`?
http://reviews.llvm.org/D11789
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
