[prev in list] [next in list] [prev in thread] [next in thread] 

List:       cfe-commits
Subject:    Re: [PATCH] D11789: Modify DeclaratorChuck::getFunction to be passed an Exception Specification Sour
From:       Richard Smith via cfe-commits <cfe-commits () lists ! llvm ! org>
Date:       2015-08-06 23:23:27
Message-ID: 013eefebf0227e6653d9f0e6ed640354 () localhost ! localdomain
[Download RAW message or body]

rsmith added inline comments.

================
Comment at: include/clang/Sema/DeclSpec.h:1262
@@ +1261,3 @@
+
+    /// \brief The end location of the keyword introducing the spec, if any.
+    unsigned ExceptionSpecLocEnd;
----------------
Drop bogus "keyword introducing the" here, and drop either the same words from the \
previous comment or the word "beginning".

================
Comment at: lib/Sema/SemaDecl.cpp:7450
@@ +7449,3 @@
+        SourceRange Range;
+        if (FPT->hasExceptionSpec() && D.isFunctionDeclarator()) {
+          Range = D.getFunctionTypeInfo().getExceptionSpecRange();
----------------
The `D.isFunctionDeclarator()` check should be nested within this, as should the \
`Range` variable:

  if (FPT->hasExceptionSpec()) {
    SourceRange Range;
    if (D.isFunctionDeclarator())
      Range = D.getFunctionTypeInfo().getExceptionSpecRange();
    PartialDiagnostic PD = ...

================
Comment at: lib/Sema/SemaDecl.cpp:7454-7456
@@ +7453,5 @@
+              PDiag(diag::err_function_concept_exception_spec);
+          if (Range.isValid()) {
+            PD << FixItHint::CreateRemoval(Range);
+          }
+          Diag(NewFD->getLocation(), PD);
----------------
You don't need this `if`; a `FixItHint` with an invalid range has no effect.

================
Comment at: test/SemaCXX/cxx-concept-declaration.cpp:9-10
@@ -8,1 +8,4 @@
 
+template<typename T> concept bool C3() { return true; }
+static_assert(noexcept(C3<int>()), "function concept should be treated as if \
noexcept(true) specified"); +
----------------
This isn't quite enough; `noexcept` will also return `true` if the expression is a \
constant expression. Maybe replace the `return true` with `return (throw 0, true)`.


http://reviews.llvm.org/D11789



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[prev in list] [next in list] [prev in thread] [next in thread]