[PATCH] D20428: Tracking exception specification source locations

[Malcolm Parsons via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL291771: Tracking exception specification source locations 
(authored by malcolm.parsons).

Changed prior to commit:
  https://reviews.llvm.org/D20428?vs=83816=84127#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D20428

Files:
  cfe/trunk/include/clang/AST/Decl.h
  cfe/trunk/include/clang/AST/TypeLoc.h
  cfe/trunk/lib/AST/Decl.cpp
  cfe/trunk/lib/Parse/ParseDeclCXX.cpp
  cfe/trunk/lib/Sema/SemaType.cpp
  cfe/trunk/lib/Sema/TreeTransform.h
  cfe/trunk/lib/Serialization/ASTReader.cpp
  cfe/trunk/lib/Serialization/ASTWriter.cpp
  cfe/trunk/unittests/AST/SourceLocationTest.cpp

Index: cfe/trunk/lib/Serialization/ASTWriter.cpp
===
--- cfe/trunk/lib/Serialization/ASTWriter.cpp
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp
@@ -629,6 +629,7 @@
   Record.AddSourceLocation(TL.getLocalRangeBegin());
   Record.AddSourceLocation(TL.getLParenLoc());
   Record.AddSourceLocation(TL.getRParenLoc());
+  Record.AddSourceRange(TL.getExceptionSpecRange());
   Record.AddSourceLocation(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i)
 Record.AddDeclRef(TL.getParam(i));
Index: cfe/trunk/lib/Serialization/ASTReader.cpp
===
--- cfe/trunk/lib/Serialization/ASTReader.cpp
+++ cfe/trunk/lib/Serialization/ASTReader.cpp
@@ -5990,6 +5990,8 @@
   TL.setLocalRangeBegin(ReadSourceLocation());
   TL.setLParenLoc(ReadSourceLocation());
   TL.setRParenLoc(ReadSourceLocation());
+  TL.setExceptionSpecRange(SourceRange(Reader->ReadSourceLocation(*F, Record, Idx),
+   Reader->ReadSourceLocation(*F, Record, Idx)));
   TL.setLocalRangeEnd(ReadSourceLocation());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i) {
 TL.setParam(i, Reader->ReadDeclAs(*F, Record, Idx));
Index: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
===
--- cfe/trunk/lib/Parse/ParseDeclCXX.cpp
+++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp
@@ -3544,7 +3544,7 @@
   Actions.CheckBooleanCondition(KeywordLoc, NoexceptExpr.get());
   NoexceptRange = SourceRange(KeywordLoc, T.getCloseLocation());
 } else {
-  NoexceptType = EST_None;
+  NoexceptType = EST_BasicNoexcept;
 }
   } else {
 // There is no argument.
Index: cfe/trunk/lib/AST/Decl.cpp
===
--- cfe/trunk/lib/AST/Decl.cpp
+++ cfe/trunk/lib/AST/Decl.cpp
@@ -2990,6 +2990,18 @@
   return RTRange;
 }
 
+SourceRange FunctionDecl::getExceptionSpecSourceRange() const {
+  const TypeSourceInfo *TSI = getTypeSourceInfo();
+  if (!TSI)
+return SourceRange();
+  FunctionTypeLoc FTL =
+TSI->getTypeLoc().IgnoreParens().getAs();
+  if (!FTL)
+return SourceRange();
+
+  return FTL.getExceptionSpecRange();
+}
+
 const Attr *FunctionDecl::getUnusedResultAttr() const {
   QualType RetType = getReturnType();
   if (RetType->isRecordType()) {
Index: cfe/trunk/lib/Sema/SemaType.cpp
===
--- cfe/trunk/lib/Sema/SemaType.cpp
+++ cfe/trunk/lib/Sema/SemaType.cpp
@@ -5263,7 +5263,7 @@
 ParmVarDecl *Param = cast(FTI.Params[i].Param);
 TL.setParam(tpi++, Param);
   }
-  // FIXME: exception specs
+  TL.setExceptionSpecRange(FTI.getExceptionSpecRange());
 }
 void VisitParenTypeLoc(ParenTypeLoc TL) {
   assert(Chunk.Kind == DeclaratorChunk::Paren);
Index: cfe/trunk/lib/Sema/TreeTransform.h
===
--- cfe/trunk/lib/Sema/TreeTransform.h
+++ cfe/trunk/lib/Sema/TreeTransform.h
@@ -5023,6 +5023,7 @@
   NewTL.setLocalRangeBegin(TL.getLocalRangeBegin());
   NewTL.setLParenLoc(TL.getLParenLoc());
   NewTL.setRParenLoc(TL.getRParenLoc());
+  NewTL.setExceptionSpecRange(TL.getExceptionSpecRange());
   NewTL.setLocalRangeEnd(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = NewTL.getNumParams(); i != e; ++i)
 NewTL.setParam(i, ParamDecls[i]);
Index: cfe/trunk/unittests/AST/SourceLocationTest.cpp
===
--- cfe/trunk/unittests/AST/SourceLocationTest.cpp
+++ cfe/trunk/unittests/AST/SourceLocationTest.cpp
@@ -670,5 +670,72 @@
   Language::Lang_CXX11));
 }
 
+class ExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const TypeLoc ) override {
+auto T =
+  Node.getUnqualifiedLoc().castAs();
+assert(!T.isNull());
+return T.getExceptionSpecRange();
+  }
+};
+
+class ParmVarExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const ParmVarDecl ) override {
+if (const TypeSourceInfo *TSI = Node.getTypeSourceInfo()) {
+  TypeLoc TL = TSI->getTypeLoc();
+  if (TL.getType()->isPointerType()) {

[PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D20428#644007, @hintonda wrote:

> Sorry, but I do not have commit access.


Ah! My apologies, I thought you did.

@malcolm.parsons, can you perform the commit? I'm traveling currently and can't 
do it myself right now.


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Sorry, but I do not have commit access.


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added a comment.

In https://reviews.llvm.org/D20428#643973, @aaron.ballman wrote:

> In https://reviews.llvm.org/D20428#643339, @hintonda wrote:
>
> > If you want this included in the 4.0 release, it needs to get in before 
> > they branch tomorrow.
>
>
> Continues to LGTM.


Does Don have commit access?


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D20428#643339, @hintonda wrote:

> If you want this included in the 4.0 release, it needs to get in before they 
> branch tomorrow.


Continues to LGTM.


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

If you want this included in the 4.0 release, it needs to get in before they 
branch tomorrow.


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[don hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 83816.
hintonda added a comment.

- Address comments.


https://reviews.llvm.org/D20428

Files:
  include/clang/AST/Decl.h
  include/clang/AST/TypeLoc.h
  lib/AST/Decl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaType.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  unittests/AST/SourceLocationTest.cpp

Index: unittests/AST/SourceLocationTest.cpp
===
--- unittests/AST/SourceLocationTest.cpp
+++ unittests/AST/SourceLocationTest.cpp
@@ -670,5 +670,72 @@
   Language::Lang_CXX11));
 }
 
+class ExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const TypeLoc ) override {
+auto T =
+  Node.getUnqualifiedLoc().castAs();
+assert(!T.isNull());
+return T.getExceptionSpecRange();
+  }
+};
+
+class ParmVarExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const ParmVarDecl ) override {
+if (const TypeSourceInfo *TSI = Node.getTypeSourceInfo()) {
+  TypeLoc TL = TSI->getTypeLoc();
+  if (TL.getType()->isPointerType()) {
+TL = TL.getNextTypeLoc().IgnoreParens();
+if (auto FPTL = TL.getAs()) {
+  return FPTL.getExceptionSpecRange();
+}
+  }
+}
+return SourceRange();
+  }
+};
+
+TEST(FunctionDecl, ExceptionSpecifications) {
+  ExceptionSpecRangeVerifier Verifier;
+
+  Verifier.expectRange(1, 10, 1, 16);
+  EXPECT_TRUE(Verifier.match("void f() throw();\n", loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 34);
+  EXPECT_TRUE(Verifier.match("void f() throw(void(void) throw());\n",
+ loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 19);
+  std::vector Args;
+  Args.push_back("-fms-extensions");
+  EXPECT_TRUE(Verifier.match("void f() throw(...);\n", loc(functionType()),
+ Args, Language::Lang_CXX));
+
+  Verifier.expectRange(1, 10, 1, 10);
+  EXPECT_TRUE(Verifier.match("void f() noexcept;\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 24);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(false);\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 32);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(noexcept(1+1));\n",
+ loc(functionType()), Language::Lang_CXX11));
+
+  ParmVarExceptionSpecRangeVerifier Verifier2;
+  Verifier2.expectRange(1, 25, 1, 31);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) throw());\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType();
+
+  Verifier2.expectRange(1, 25, 1, 38);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) noexcept(true));\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType())),
+  Language::Lang_CXX11));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -629,6 +629,7 @@
   Record.AddSourceLocation(TL.getLocalRangeBegin());
   Record.AddSourceLocation(TL.getLParenLoc());
   Record.AddSourceLocation(TL.getRParenLoc());
+  Record.AddSourceRange(TL.getExceptionSpecRange());
   Record.AddSourceLocation(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i)
 Record.AddDeclRef(TL.getParam(i));
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5990,6 +5990,8 @@
   TL.setLocalRangeBegin(ReadSourceLocation());
   TL.setLParenLoc(ReadSourceLocation());
   TL.setRParenLoc(ReadSourceLocation());
+  TL.setExceptionSpecRange(SourceRange(Reader->ReadSourceLocation(*F, Record, Idx),
+   Reader->ReadSourceLocation(*F, Record, Idx)));
   TL.setLocalRangeEnd(ReadSourceLocation());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i) {
 TL.setParam(i, Reader->ReadDeclAs(*F, Record, Idx));
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -5023,6 +5023,7 @@
   NewTL.setLocalRangeBegin(TL.getLocalRangeBegin());
   NewTL.setLParenLoc(TL.getLParenLoc());
   NewTL.setRParenLoc(TL.getRParenLoc());
+  NewTL.setExceptionSpecRange(TL.getExceptionSpecRange());
   NewTL.setLocalRangeEnd(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = NewTL.getNumParams(); i != e; ++i)
 NewTL.setParam(i, ParamDecls[i]);
Index: 

[PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D20428#641416, @hintonda wrote:

> Aaron, I've got this version in a branch, so if you like, I'd be happy to 
> apply Malcolm's suggestions.


Please do; they're good suggestions.


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Aaron, I've got this version in a branch, so if you like, I'd be happy to apply 
Malcolm's suggestions.


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[Malcolm Parsons via Phabricator via cfe-commits]

malcolm.parsons added inline comments.



Comment at: include/clang/AST/TypeLoc.h:1355
+  bool hasExceptionSpec() const {
+if (auto FPT = dyn_cast(getTypePtr())) {
+  return FPT->hasExceptionSpec();

`auto *`



Comment at: include/clang/AST/TypeLoc.h:1443
+if (hasExceptionSpec())
+  setExceptionSpecRange(SourceRange(Loc));
   }

`SourceRange`'s constructor from `SourceLocation` is implicit.


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I think that these changes look good to me, and I noticed Richard signed off on 
the other review for the exception spec changes (thank you for working on 
that!), so I think this is ready to commit.

In https://reviews.llvm.org/D20428#641068, @hintonda wrote:

> Richard, is it okay to commit this?


If Richard does not respond today, then yes, it's fine to commit (it's had 
relatively extensive review already, and we can correct any outstanding issues 
post-commit).


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Richard, is it okay to commit this?


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

Great, thanks.  Spoke to Richard about it last night and believe he's 
comfortable with this change, but I'll let him weigh in...


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[don hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 83346.
hintonda added a comment.

- Fix compile errors.
- When noexcept expr is invalid, set NoexceptType to EST_BasicNoexcept


https://reviews.llvm.org/D20428

Files:
  include/clang/AST/Decl.h
  include/clang/AST/TypeLoc.h
  lib/AST/Decl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaType.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  unittests/AST/SourceLocationTest.cpp

Index: unittests/AST/SourceLocationTest.cpp
===
--- unittests/AST/SourceLocationTest.cpp
+++ unittests/AST/SourceLocationTest.cpp
@@ -670,5 +670,72 @@
   Language::Lang_CXX11));
 }
 
+class ExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const TypeLoc ) override {
+auto T =
+  Node.getUnqualifiedLoc().castAs();
+assert(!T.isNull());
+return T.getExceptionSpecRange();
+  }
+};
+
+class ParmVarExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const ParmVarDecl ) override {
+if (const TypeSourceInfo *TSI = Node.getTypeSourceInfo()) {
+  TypeLoc TL = TSI->getTypeLoc();
+  if (TL.getType()->isPointerType()) {
+TL = TL.getNextTypeLoc().IgnoreParens();
+if (auto FPTL = TL.getAs()) {
+  return FPTL.getExceptionSpecRange();
+}
+  }
+}
+return SourceRange();
+  }
+};
+
+TEST(FunctionDecl, ExceptionSpecifications) {
+  ExceptionSpecRangeVerifier Verifier;
+
+  Verifier.expectRange(1, 10, 1, 16);
+  EXPECT_TRUE(Verifier.match("void f() throw();\n", loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 34);
+  EXPECT_TRUE(Verifier.match("void f() throw(void(void) throw());\n",
+ loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 19);
+  std::vector Args;
+  Args.push_back("-fms-extensions");
+  EXPECT_TRUE(Verifier.match("void f() throw(...);\n", loc(functionType()),
+ Args, Language::Lang_CXX));
+
+  Verifier.expectRange(1, 10, 1, 10);
+  EXPECT_TRUE(Verifier.match("void f() noexcept;\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 24);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(false);\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 32);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(noexcept(1+1));\n",
+ loc(functionType()), Language::Lang_CXX11));
+
+  ParmVarExceptionSpecRangeVerifier Verifier2;
+  Verifier2.expectRange(1, 25, 1, 31);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) throw());\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType();
+
+  Verifier2.expectRange(1, 25, 1, 38);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) noexcept(true));\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType())),
+  Language::Lang_CXX11));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -629,6 +629,7 @@
   Record.AddSourceLocation(TL.getLocalRangeBegin());
   Record.AddSourceLocation(TL.getLParenLoc());
   Record.AddSourceLocation(TL.getRParenLoc());
+  Record.AddSourceRange(TL.getExceptionSpecRange());
   Record.AddSourceLocation(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i)
 Record.AddDeclRef(TL.getParam(i));
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5990,6 +5990,8 @@
   TL.setLocalRangeBegin(ReadSourceLocation());
   TL.setLParenLoc(ReadSourceLocation());
   TL.setRParenLoc(ReadSourceLocation());
+  TL.setExceptionSpecRange(SourceRange(Reader->ReadSourceLocation(*F, Record, Idx),
+   Reader->ReadSourceLocation(*F, Record, Idx)));
   TL.setLocalRangeEnd(ReadSourceLocation());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i) {
 TL.setParam(i, Reader->ReadDeclAs(*F, Record, Idx));
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -5023,6 +5023,7 @@
   NewTL.setLocalRangeBegin(TL.getLocalRangeBegin());
   NewTL.setLParenLoc(TL.getLParenLoc());
   NewTL.setRParenLoc(TL.getRParenLoc());
+  NewTL.setExceptionSpecRange(TL.getExceptionSpecRange());
   NewTL.setLocalRangeEnd(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = NewTL.getNumParams(); 

[PATCH] D20428: Tracking exception specification source locations

[don hinton via Phabricator via cfe-commits]

hintonda added a comment.

The problem is that when a noexcept(bool expr) specification is invalid, e.g., 
bad bool expr, the NoexceptType in Parser::tryParseExceptionSpecification is 
set to EST_None and returned.  This will mean the FunctionDecl type won't have 
an exception TypeLoc, but the TypeSourceInfo type in 
ASTContext::adjustExceptionSpec will, which will trigger the assert in due to 
different sizes.

The easy fix is to set NoexceptType = EST_BasicNoexcept which will cause all 
tests to pass.  The other option would be to leave it as EST_ComputedNoexcept, 
but that would cause a crash later during template instantiation.

Here's a patch (not sure I can uploade it):

  diff --git a/lib/Parse/ParseDeclCXX.cpp b/lib/Parse/ParseDeclCXX.cpp
  index 4002b09..7cfb8d5 100644
  --- a/lib/Parse/ParseDeclCXX.cpp
  +++ b/lib/Parse/ParseDeclCXX.cpp
  @@ -3545,7 +3545,7 @@ Parser::tryParseExceptionSpecification(bool Delayed,
 Actions.CheckBooleanCondition(KeywordLoc, NoexceptExpr.get());
 NoexceptRange = SourceRange(KeywordLoc, T.getCloseLocation());
   } else {
  -  NoexceptType = EST_None;
  +  NoexceptType = EST_BasicNoexcept;
   }
 } else {
   // There is no argument.




https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D20428#564511, @sbarzowski wrote:

> What's happening here? It's accepted, but not merged and the last activity is 
> 3 months old while my change is waiting for it. Can I help somehow?


It was accepted, but then caused build failures. I started correcting it, but 
ran into problems properly getting `TypeLocBuilder` to generate the proper type 
location information. Since then, I've ran out of funding to work on the 
problem and haven't had the spare time to pick it back up again. If someone 
would like to work on this patch in the meantime, I certainly would not be 
offended (and I'd be happy to help review).


https://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[Stanisław Barzowski via cfe-commits]

sbarzowski added a comment.

What's happening here? It's accepted, but not merged and the last activity is 3 
months old while my change is waiting for it. Can I help somehow?


https://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

On Fri, Jun 24, 2016 at 8:39 AM, Richard Smith  wrote:
> rsmith added a comment.
>
> Ah right, we were (intentionally, but unfortunately) making an assumption 
> that the `TypeLoc` data layout doesn't change when the exception spec of a 
> function is updated. You'd need to make yourself a `TypeLocBuilder`, copy the 
> relevant data, and update the exception spec loc.

Would that look something like what we do in SemaType.cpp
GetFullTypeForDeclarator()? i.e.,

if (TInfo) {
  TypeLocBuilder TLB;
  TLB.pushFullCopy(TInfo->getTypeLoc());
  ObjCObjectPointerTypeLoc TLoc = TLB.push(T);
  TLoc.setStarLoc(FixitLoc);
  TInfo = TLB.getTypeSourceInfo(Context, T);
}

~Aaron

>
> Where are we adjusting the exception specification on the type source info? 
> That (changing the type-info-as-written) seems like a somewhat unusual thing 
> to do implicitly.
>
>
> http://reviews.llvm.org/D20428
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

On Fri, Jun 24, 2016 at 8:39 AM, Richard Smith  wrote:
> rsmith added a comment.
>
> Ah right, we were (intentionally, but unfortunately) making an assumption 
> that the `TypeLoc` data layout doesn't change when the exception spec of a 
> function is updated. You'd need to make yourself a `TypeLocBuilder`, copy the 
> relevant data, and update the exception spec loc.
>
> Where are we adjusting the exception specification on the type source info? 
> That (changing the type-info-as-written) seems like a somewhat unusual thing 
> to do implicitly.

ActOnFunctionDeclarator when dealing with concepts ([dcl.spec.concept]p1)
delayed exception specs
Any time we explicitly call UpdateExceptionSpec() (template
instantiation and a few other places).

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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D20428#452680, @hintonda wrote:

> The comment says to rebuild TypeSourceInfo, but isn't that what this does?
>
>   if (TSInfo->getType() != FD->getType())
> Updated = getFunctionTypeWithExceptionSpec(*this, TSInfo->getType(), ESI);
>   TSInfo->overrideType(Updated);
>   


That is only overriding the `QualType` stored in the `TypeSourceInfo`, but it 
does not rebuild the type source information as a whole (which includes the 
`FunctionTypeLoc`). The problem is that we need a new TSI that has sufficient 
space to store the exception specification source range in the case the 
existing source info does not have it. What's more, this function doesn't 
currently have that information (I believe it needs to be passed in as a 
parameter) to store in the adjusted type info.


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

Richard, with the following test case, my patch currently fails an assertion in 
`ASTContext::adjustExceptionSpec()` that I want to solve before committing:

  template void f7() {
struct S { void g() noexcept(undefined_val); };  // expected-error{{use of 
undeclared identifier 'undefined_val'}}
  }
  template void f7();

What is the correct way to rebuild the type source information? Would it be 
correct to call CreateTypeSourceInfo(Updated) to get a new TypeSourceInfo 
object of the proper size, then initializeFullCopy() the new type location 
object from the old one, and call setExceptionSpecRange() to update the source 
range information on the new type loc object? (The range would have to be 
passed to adjustExceptionSpec().) Or is there a better way to perform this 
rebuilding?



Comment at: lib/AST/Decl.cpp:2938-2948
@@ -2937,1 +2937,13 @@
 
+SourceRange FunctionDecl::getExceptionSpecSourceRange() const {
+  const TypeSourceInfo *TSI = getTypeSourceInfo();
+  if (!TSI)
+return SourceRange();
+  FunctionTypeLoc FTL =
+TSI->getTypeLoc().IgnoreParens().getAs();
+  if (!FTL)
+return SourceRange();
+
+  return FTL.getExceptionSpecRange();
+}
+

rsmith wrote:
> Can you factor out a function to get the `FunctionTypeLoc` from a 
> `FunctionDecl`, when there is one (preferably as a separate change)? This is 
> duplicated in a few places now (you can find some more by searching for 
> `getAs` in Sema), and looks slightly wrong here (we 
> should skip calling convention attributes as well as parens).
Yup, I can do that.


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Richard Smith via cfe-commits]

rsmith accepted this revision.
This revision is now accepted and ready to land.


Comment at: lib/AST/Decl.cpp:2938-2948
@@ -2937,1 +2937,13 @@
 
+SourceRange FunctionDecl::getExceptionSpecSourceRange() const {
+  const TypeSourceInfo *TSI = getTypeSourceInfo();
+  if (!TSI)
+return SourceRange();
+  FunctionTypeLoc FTL =
+TSI->getTypeLoc().IgnoreParens().getAs();
+  if (!FTL)
+return SourceRange();
+
+  return FTL.getExceptionSpecRange();
+}
+

Can you factor out a function to get the `FunctionTypeLoc` from a 
`FunctionDecl`, when there is one (preferably as a separate change)? This is 
duplicated in a few places now (you can find some more by searching for 
`getAs` in Sema), and looks slightly wrong here (we 
should skip calling convention attributes as well as parens).


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[don hinton via cfe-commits]

hintonda added a comment.

The comment says to rebuild TypeSourceInfo, but isn't that what this does?

  if (TSInfo->getType() != FD->getType())
Updated = getFunctionTypeWithExceptionSpec(*this, TSInfo->getType(), ESI);
  TSInfo->overrideType(Updated);

If so, could you fix this by either removing the assert or moving it below

  TSInfo->overrideType(Updated);


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

One thing this patch does not do but needs to is fix 
`ASTContext::adjustExceptionSpec()` (Thanks to Don Hinton for pointing this out 
off-list!), however, I am at a bit of a loss for how best to rebuild the type 
location.

Would it be correct to call `CreateTypeSourceInfo(Updated)` to get a new 
TypeSourceInfo object of the proper size, then `initializeFullCopy()` the new 
type location object from the old one, and call `setExceptionSpecRange()` to 
update the source range information on the new type loc object? (The range 
would have to be passed to `adjustExceptionSpec()`.) Or is there a better way 
to perform this rebuilding?


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

@rsmith: Ping


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[don hinton via cfe-commits]

hintonda added inline comments.


Comment at: lib/Parse/ParseDeclCXX.cpp:3403-3428
@@ -3402,6 +3402,7 @@
   // If we already had a dynamic specification, parse the noexcept for,
   // recovery, but emit a diagnostic and don't store the results.
-  SourceRange NoexceptRange;
+  SourceRange NoexceptRange(Tok.getLocation(),
+Tok.getEndLoc().getLocWithOffset(-1));
   ExceptionSpecificationType NoexceptType = EST_None;
 
   SourceLocation KeywordLoc = ConsumeToken();

@@ -3424,6 +3425,5 @@
   } else {
 // There is no argument.
 NoexceptType = EST_BasicNoexcept;
-NoexceptRange = SourceRange(KeywordLoc, KeywordLoc);
   }
 

hintonda wrote:
> aaron.ballman wrote:
> > hintonda wrote:
> > > The range for a single token is a single location.  The problem is your 
> > > test.  The range for "throw()" ends at the start of the ')' token.  For 
> > > "noexcept", the beginning and end are the same location, e.g., here's how 
> > > "int" is tested:
> > > 
> > > ```
> > > TEST(MatchVerifier, ParseError) {
> > >   LocationVerifier Verifier;
> > >   Verifier.expectLocation(1, 1);
> > >   EXPECT_FALSE(Verifier.match("int i", varDecl()));
> > > }
> > > ```
> > > 
> > > I think you should use this instead:
> > > 
> > > ```
> > > SourceRange NoexceptRange(Tok.getLocation());
> > > ```
> > Ah, how interesting; I would have assumed the range would be the full range 
> > of the source involved, but I forgot, that's what char ranges are for.
> Pasted the wrong test -- this one does the range:
> 
> ```
> TEST(RangeVerifier, WrongRange) {
>   RangeVerifier Verifier;
>   Verifier.expectRange(1, 1, 1, 1);
>   EXPECT_FALSE(Verifier.match("int i;", varDecl()));
> }
> ```
Sorry, looked at wrong tests -- should have looked at multi-character ending 
tokens. 

```
  NoexceptRange = SourceRange(
Tok.getLocation(), Tok.getLocation().getLocWithOffset(Tok.getLength()));
```


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman updated this revision to Diff 58804.
aaron.ballman marked 3 inline comments as done.
aaron.ballman added a comment.

Updating based on further review comments.


http://reviews.llvm.org/D20428

Files:
  include/clang/AST/Decl.h
  include/clang/AST/TypeLoc.h
  lib/AST/Decl.cpp
  lib/Sema/SemaType.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  unittests/AST/SourceLocationTest.cpp

Index: unittests/AST/SourceLocationTest.cpp
===
--- unittests/AST/SourceLocationTest.cpp
+++ unittests/AST/SourceLocationTest.cpp
@@ -580,5 +580,72 @@
   Language::Lang_CXX11));
 }
 
+class ExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const TypeLoc ) override {
+auto T =
+  Node.getUnqualifiedLoc().castAs();
+assert(!T.isNull());
+return T.getExceptionSpecRange();
+  }
+};
+
+class ParmVarExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const ParmVarDecl ) override {
+if (const TypeSourceInfo *TSI = Node.getTypeSourceInfo()) {
+  TypeLoc TL = TSI->getTypeLoc();
+  if (TL.getType()->isPointerType()) {
+TL = TL.getNextTypeLoc().IgnoreParens();
+if (auto FPTL = TL.getAs()) {
+  return FPTL.getExceptionSpecRange();
+}
+  }
+}
+return SourceRange();
+  }
+};
+
+TEST(FunctionDecl, ExceptionSpecifications) {
+  ExceptionSpecRangeVerifier Verifier;
+
+  Verifier.expectRange(1, 10, 1, 16);
+  EXPECT_TRUE(Verifier.match("void f() throw();\n", loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 34);
+  EXPECT_TRUE(Verifier.match("void f() throw(void(void) throw());\n",
+ loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 19);
+  std::vector Args;
+  Args.push_back("-fms-extensions");
+  EXPECT_TRUE(Verifier.match("void f() throw(...);\n", loc(functionType()),
+ Args, Language::Lang_CXX));
+
+  Verifier.expectRange(1, 10, 1, 10);
+  EXPECT_TRUE(Verifier.match("void f() noexcept;\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 24);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(false);\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 32);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(noexcept(1+1));\n",
+ loc(functionType()), Language::Lang_CXX11));
+
+  ParmVarExceptionSpecRangeVerifier Verifier2;
+  Verifier2.expectRange(1, 25, 1, 31);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) throw());\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType();
+
+  Verifier2.expectRange(1, 25, 1, 38);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) noexcept(true));\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType())),
+  Language::Lang_CXX11));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -572,6 +572,7 @@
   Record.AddSourceLocation(TL.getLocalRangeBegin());
   Record.AddSourceLocation(TL.getLParenLoc());
   Record.AddSourceLocation(TL.getRParenLoc());
+  Record.AddSourceRange(TL.getExceptionSpecRange());
   Record.AddSourceLocation(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i)
 Record.AddDeclRef(TL.getParam(i));
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5812,6 +5812,8 @@
   TL.setLocalRangeBegin(ReadSourceLocation(Record, Idx));
   TL.setLParenLoc(ReadSourceLocation(Record, Idx));
   TL.setRParenLoc(ReadSourceLocation(Record, Idx));
+  TL.setExceptionSpecRange(SourceRange(ReadSourceLocation(Record, Idx),
+   ReadSourceLocation(Record, Idx)));
   TL.setLocalRangeEnd(ReadSourceLocation(Record, Idx));
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i) {
 TL.setParam(i, ReadDeclAs(Record, Idx));
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4942,6 +4942,7 @@
   NewTL.setLocalRangeBegin(TL.getLocalRangeBegin());
   NewTL.setLParenLoc(TL.getLParenLoc());
   NewTL.setRParenLoc(TL.getRParenLoc());
+  NewTL.setExceptionSpecRange(TL.getExceptionSpecRange());
   NewTL.setLocalRangeEnd(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = NewTL.getNumParams(); i != e; ++i)
  

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: lib/Parse/ParseDeclCXX.cpp:3403-3428
@@ -3402,6 +3402,7 @@
   // If we already had a dynamic specification, parse the noexcept for,
   // recovery, but emit a diagnostic and don't store the results.
-  SourceRange NoexceptRange;
+  SourceRange NoexceptRange(Tok.getLocation(),
+Tok.getEndLoc().getLocWithOffset(-1));
   ExceptionSpecificationType NoexceptType = EST_None;
 
   SourceLocation KeywordLoc = ConsumeToken();

@@ -3424,6 +3425,5 @@
   } else {
 // There is no argument.
 NoexceptType = EST_BasicNoexcept;
-NoexceptRange = SourceRange(KeywordLoc, KeywordLoc);
   }
 

hintonda wrote:
> The range for a single token is a single location.  The problem is your test. 
>  The range for "throw()" ends at the start of the ')' token.  For "noexcept", 
> the beginning and end are the same location, e.g., here's how "int" is tested:
> 
> ```
> TEST(MatchVerifier, ParseError) {
>   LocationVerifier Verifier;
>   Verifier.expectLocation(1, 1);
>   EXPECT_FALSE(Verifier.match("int i", varDecl()));
> }
> ```
> 
> I think you should use this instead:
> 
> ```
> SourceRange NoexceptRange(Tok.getLocation());
> ```
Ah, how interesting; I would have assumed the range would be the full range of 
the source involved, but I forgot, that's what char ranges are for.


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[don hinton via cfe-commits]

hintonda added inline comments.


Comment at: lib/Parse/ParseDeclCXX.cpp:3403-3428
@@ -3402,6 +3402,7 @@
   // If we already had a dynamic specification, parse the noexcept for,
   // recovery, but emit a diagnostic and don't store the results.
-  SourceRange NoexceptRange;
+  SourceRange NoexceptRange(Tok.getLocation(),
+Tok.getEndLoc().getLocWithOffset(-1));
   ExceptionSpecificationType NoexceptType = EST_None;
 
   SourceLocation KeywordLoc = ConsumeToken();

@@ -3424,6 +3425,5 @@
   } else {
 // There is no argument.
 NoexceptType = EST_BasicNoexcept;
-NoexceptRange = SourceRange(KeywordLoc, KeywordLoc);
   }
 

hintonda wrote:
> The range for a single token is a single location.  The problem is your test. 
>  The range for "throw()" ends at the start of the ')' token.  For "noexcept", 
> the beginning and end are the same location, e.g., here's how "int" is tested:
> 
> ```
> TEST(MatchVerifier, ParseError) {
>   LocationVerifier Verifier;
>   Verifier.expectLocation(1, 1);
>   EXPECT_FALSE(Verifier.match("int i", varDecl()));
> }
> ```
> 
> I think you should use this instead:
> 
> ```
> SourceRange NoexceptRange(Tok.getLocation());
> ```
Pasted the wrong test -- this one does the range:

```
TEST(RangeVerifier, WrongRange) {
  RangeVerifier Verifier;
  Verifier.expectRange(1, 1, 1, 1);
  EXPECT_FALSE(Verifier.match("int i;", varDecl()));
}
```


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[don hinton via cfe-commits]

hintonda added inline comments.


Comment at: lib/Parse/ParseDeclCXX.cpp:3403-3428
@@ -3402,6 +3402,7 @@
   // If we already had a dynamic specification, parse the noexcept for,
   // recovery, but emit a diagnostic and don't store the results.
-  SourceRange NoexceptRange;
+  SourceRange NoexceptRange(Tok.getLocation(),
+Tok.getEndLoc().getLocWithOffset(-1));
   ExceptionSpecificationType NoexceptType = EST_None;
 
   SourceLocation KeywordLoc = ConsumeToken();

@@ -3424,6 +3425,5 @@
   } else {
 // There is no argument.
 NoexceptType = EST_BasicNoexcept;
-NoexceptRange = SourceRange(KeywordLoc, KeywordLoc);
   }
 

The range for a single token is a single location.  The problem is your test.  
The range for "throw()" ends at the start of the ')' token.  For "noexcept", 
the beginning and end are the same location, e.g., here's how "int" is tested:

```
TEST(MatchVerifier, ParseError) {
  LocationVerifier Verifier;
  Verifier.expectLocation(1, 1);
  EXPECT_FALSE(Verifier.match("int i", varDecl()));
}
```

I think you should use this instead:

```
SourceRange NoexceptRange(Tok.getLocation());
```


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman updated this revision to Diff 58784.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Updated based on review comments.


http://reviews.llvm.org/D20428

Files:
  include/clang/AST/Decl.h
  include/clang/AST/TypeLoc.h
  lib/AST/Decl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaType.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  unittests/AST/SourceLocationTest.cpp

Index: unittests/AST/SourceLocationTest.cpp
===
--- unittests/AST/SourceLocationTest.cpp
+++ unittests/AST/SourceLocationTest.cpp
@@ -580,5 +580,72 @@
   Language::Lang_CXX11));
 }
 
+class ExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const TypeLoc ) override {
+auto T =
+  Node.getUnqualifiedLoc().castAs();
+assert(!T.isNull());
+return T.getExceptionSpecRange();
+  }
+};
+
+class ParmVarExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const ParmVarDecl ) override {
+if (const TypeSourceInfo *TSI = Node.getTypeSourceInfo()) {
+  TypeLoc TL = TSI->getTypeLoc();
+  if (TL.getType()->isPointerType()) {
+TL = TL.getNextTypeLoc().IgnoreParens();
+if (auto FPTL = TL.getAs()) {
+  return FPTL.getExceptionSpecRange();
+}
+  }
+}
+return SourceRange();
+  }
+};
+
+TEST(FunctionDecl, ExceptionSpecifications) {
+  ExceptionSpecRangeVerifier Verifier;
+
+  Verifier.expectRange(1, 10, 1, 16);
+  EXPECT_TRUE(Verifier.match("void f() throw();\n", loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 34);
+  EXPECT_TRUE(Verifier.match("void f() throw(void(void) throw());\n",
+ loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 19);
+  std::vector Args;
+  Args.push_back("-fms-extensions");
+  EXPECT_TRUE(Verifier.match("void f() throw(...);\n", loc(functionType()),
+ Args, Language::Lang_CXX));
+
+  Verifier.expectRange(1, 10, 1, 17);
+  EXPECT_TRUE(Verifier.match("void f() noexcept;\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 24);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(false);\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 32);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(noexcept(1+1));\n",
+ loc(functionType()), Language::Lang_CXX11));
+
+  ParmVarExceptionSpecRangeVerifier Verifier2;
+  Verifier2.expectRange(1, 25, 1, 31);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) throw());\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType();
+
+  Verifier2.expectRange(1, 25, 1, 38);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) noexcept(true));\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType())),
+  Language::Lang_CXX11));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -572,6 +572,7 @@
   Record.AddSourceLocation(TL.getLocalRangeBegin());
   Record.AddSourceLocation(TL.getLParenLoc());
   Record.AddSourceLocation(TL.getRParenLoc());
+  Record.AddSourceRange(TL.getExceptionSpecRange());
   Record.AddSourceLocation(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i)
 Record.AddDeclRef(TL.getParam(i));
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5812,6 +5812,8 @@
   TL.setLocalRangeBegin(ReadSourceLocation(Record, Idx));
   TL.setLParenLoc(ReadSourceLocation(Record, Idx));
   TL.setRParenLoc(ReadSourceLocation(Record, Idx));
+  TL.setExceptionSpecRange(SourceRange(ReadSourceLocation(Record, Idx),
+   ReadSourceLocation(Record, Idx)));
   TL.setLocalRangeEnd(ReadSourceLocation(Record, Idx));
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i) {
 TL.setParam(i, ReadDeclAs(Record, Idx));
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4942,6 +4942,7 @@
   NewTL.setLocalRangeBegin(TL.getLocalRangeBegin());
   NewTL.setLParenLoc(TL.getLParenLoc());
   NewTL.setRParenLoc(TL.getRParenLoc());
+  NewTL.setExceptionSpecRange(TL.getExceptionSpecRange());
   NewTL.setLocalRangeEnd(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = 

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added inline comments.


Comment at: lib/Parse/ParseDeclCXX.cpp:3403-3427
@@ -3402,5 +3402,6 @@
   // recovery, but emit a diagnostic and don't store the results.
-  SourceRange NoexceptRange;
+  SourceRange NoexceptRange(Tok.getLocation(),
+Tok.getEndLoc().getLocWithOffset(-1));
   ExceptionSpecificationType NoexceptType = EST_None;
 
   SourceLocation KeywordLoc = ConsumeToken();

@@ -3423,6 +3424,5 @@
   } else {
 // There is no argument.
 NoexceptType = EST_BasicNoexcept;
-NoexceptRange = SourceRange(KeywordLoc, KeywordLoc);
   }
 

rsmith wrote:
> This change seems strange:
> 
>  1) We should be using normal token-based ranges here; the 
> `getEndLoc().getLocWithOffset(-1)` doesn't make sense.
>  2) This specifies a range for the error case, where we produce an exception 
> specification of `EST_None`. We should not have a `NoexceptRange` if we're 
> recovering as if there were no `noexcept`.
> 
> Can you revert the changes to this file?
I agree that this code is strange, what I found was that anything other than 
getLocWithOffset would produce the wrong range (the resulting range would be 
off-by-one). I found other code that does this, but it may be equally incorrect 
if there's a better way to get a range for a token.

As for #2, I don't think that's actually the case. This is the code path that 
parses a noexcept specifier with no arguments. (Result starts as EST_None, we 
aren't delayed, there is no dynamic exception spec, the next token is noexcept, 
there is no l_paren, and so we set SpecificationRange and Result before 
returning). Without tracking the source range, the following test case fails 
because the token range has no length:
``` 
  Verifier.expectRange(1, 10, 1, 17);
  EXPECT_TRUE(Verifier.match("void f() noexcept;\n", loc(functionType()),
 Language::Lang_CXX11));
```
Is there a better way for me to get the full source range of the token? I would 
have assumed that getEndLoc() would work, but it appears to give an 
inconsistent result with other ranges that we track. e.g., noexcept(false) the 
caret points at ), throw(...) the caret points at ), but without the -1 offset 
above for void f() noexcept; the caret points at ; instead of t.


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Richard Smith via cfe-commits]

rsmith added inline comments.


Comment at: include/clang/AST/TypeLoc.h:1251
@@ -1250,2 +1250,3 @@
   SourceLocation RParenLoc;
+  SourceRange ExceptionSpecRange;
   SourceLocation LocalRangeEnd;

Can you arrange things so we only store this if there is an exception 
specification? Or, failing that, at least only store it for a 
`FunctionProtoTypeLoc`.


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Richard Smith via cfe-commits]

rsmith added a comment.

Seems reasonable to me.



Comment at: lib/Parse/ParseDeclCXX.cpp:3403-3427
@@ -3402,5 +3402,6 @@
   // recovery, but emit a diagnostic and don't store the results.
-  SourceRange NoexceptRange;
+  SourceRange NoexceptRange(Tok.getLocation(),
+Tok.getEndLoc().getLocWithOffset(-1));
   ExceptionSpecificationType NoexceptType = EST_None;
 
   SourceLocation KeywordLoc = ConsumeToken();

@@ -3423,6 +3424,5 @@
   } else {
 // There is no argument.
 NoexceptType = EST_BasicNoexcept;
-NoexceptRange = SourceRange(KeywordLoc, KeywordLoc);
   }
 

This change seems strange:

 1) We should be using normal token-based ranges here; the 
`getEndLoc().getLocWithOffset(-1)` doesn't make sense.
 2) This specifies a range for the error case, where we produce an exception 
specification of `EST_None`. We should not have a `NoexceptRange` if we're 
recovering as if there were no `noexcept`.

Can you revert the changes to this file?


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Richard, ping.


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[don hinton via cfe-commits]

hintonda added a comment.

Sure that sounds good to me.  However, I would like to learn how to write 
better ASTMatchers.

In any case, this has been a great learning experience.


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D20428#434420, @hintonda wrote:

> I can already catch all of these cases, but I can't catch this one, will this 
> catch it too?
>
>   void g(void (*fp)(void) throw()) throw();
>   ^^^
>
>
> Actually, I can catch it, but I haven't been able to create an ASTMatcher 
> that will only include FunctionDecl's -- even using functionDecl() still 
> returns all Decl's, not just FunctionDecl's.


Yes, this information is also tracked as part of my patch. I've added a few 
more test cases for it, thank you!

Assuming that @rsmith thinks my direction is reasonable for tracking source 
information, I strongly prefer to gate your patch on this one. Your custom 
parser may work fine, but I don't think it should be required since we already 
have this information from the real parser. Might as well thread the 
information through so that it can be used by tooling.


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman updated this revision to Diff 57816.
aaron.ballman added a comment.

Added test cases for parameter types that have exception specifications.


http://reviews.llvm.org/D20428

Files:
  include/clang/AST/Decl.h
  include/clang/AST/TypeLoc.h
  lib/AST/Decl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaType.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  unittests/AST/SourceLocationTest.cpp

Index: unittests/AST/SourceLocationTest.cpp
===
--- unittests/AST/SourceLocationTest.cpp
+++ unittests/AST/SourceLocationTest.cpp
@@ -580,5 +580,72 @@
   Language::Lang_CXX11));
 }
 
+class ExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const TypeLoc ) override {
+auto T =
+  Node.getUnqualifiedLoc().castAs();
+assert(!T.isNull());
+return T.getExceptionSpecRange();
+  }
+};
+
+class ParmVarExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const ParmVarDecl ) override {
+if (const TypeSourceInfo *TSI = Node.getTypeSourceInfo()) {
+  TypeLoc TL = TSI->getTypeLoc();
+  if (TL.getType()->isPointerType()) {
+TL = TL.getNextTypeLoc().IgnoreParens();
+if (auto FPTL = TL.getAs()) {
+  return FPTL.getExceptionSpecRange();
+}
+  }
+}
+return SourceRange();
+  }
+};
+
+TEST(FunctionDecl, ExceptionSpecifications) {
+  ExceptionSpecRangeVerifier Verifier;
+
+  Verifier.expectRange(1, 10, 1, 16);
+  EXPECT_TRUE(Verifier.match("void f() throw();\n", loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 34);
+  EXPECT_TRUE(Verifier.match("void f() throw(void(void) throw());\n",
+ loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 19);
+  std::vector Args;
+  Args.push_back("-fms-extensions");
+  EXPECT_TRUE(Verifier.match("void f() throw(...);\n", loc(functionType()),
+ Args, Language::Lang_CXX));
+
+  Verifier.expectRange(1, 10, 1, 17);
+  EXPECT_TRUE(Verifier.match("void f() noexcept;\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 24);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(false);\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 32);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(noexcept(1+1));\n",
+ loc(functionType()), Language::Lang_CXX11));
+
+  ParmVarExceptionSpecRangeVerifier Verifier2;
+  Verifier2.expectRange(1, 25, 1, 31);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) throw());\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType();
+
+  Verifier2.expectRange(1, 25, 1, 38);
+  EXPECT_TRUE(Verifier2.match("void g(void (*fp)(void) noexcept(true));\n",
+  parmVarDecl(hasType(pointerType(pointee(
+  parenType(innerType(functionType())),
+  Language::Lang_CXX11));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -572,6 +572,7 @@
   Record.AddSourceLocation(TL.getLocalRangeBegin());
   Record.AddSourceLocation(TL.getLParenLoc());
   Record.AddSourceLocation(TL.getRParenLoc());
+  Record.AddSourceRange(TL.getExceptionSpecRange());
   Record.AddSourceLocation(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i)
 Record.AddDeclRef(TL.getParam(i));
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5812,6 +5812,8 @@
   TL.setLocalRangeBegin(ReadSourceLocation(Record, Idx));
   TL.setLParenLoc(ReadSourceLocation(Record, Idx));
   TL.setRParenLoc(ReadSourceLocation(Record, Idx));
+  TL.setExceptionSpecRange(SourceRange(ReadSourceLocation(Record, Idx),
+   ReadSourceLocation(Record, Idx)));
   TL.setLocalRangeEnd(ReadSourceLocation(Record, Idx));
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i) {
 TL.setParam(i, ReadDeclAs(Record, Idx));
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4919,6 +4919,7 @@
   NewTL.setLocalRangeBegin(TL.getLocalRangeBegin());
   NewTL.setLParenLoc(TL.getLParenLoc());
   NewTL.setRParenLoc(TL.getRParenLoc());
+  NewTL.setExceptionSpecRange(TL.getExceptionSpecRange());
   NewTL.setLocalRangeEnd(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = NewTL.getNumParams(); i 

Re: [PATCH] D20428: Tracking exception specification source locations

[don hinton via cfe-commits]

hintonda added a comment.

I can already catch all of these cases, but I can't catch this one, will this 
catch it too?

  void g(void (*fp)(void) throw()) throw();
  ^^^


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman added a comment.

In http://reviews.llvm.org/D20428#434270, @alexfh wrote:

> In http://reviews.llvm.org/D20428#434242, @aaron.ballman wrote:
>
> > In http://reviews.llvm.org/D20428#434238, @alexfh wrote:
> >
> > > Full context diff, please.
> >
> >
> > Pardon my complete ignorance, but how? I generated the diff from svn the 
> > usual way, so I assume I've missed some step.
>
>
> There are at least two ways to do this:
>
> 1. both `git diff` and `svn diff` can be convinced to produce diffs with full 
> context: 
> http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
> 2. I find arcanist a very useful alternative to using web-interface for 
> posting diffs: 
> http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line


Hmm, so (1) it's strange that I've yet to run into this before today, and (2) 
I'm on Windows using TortoiseSVN, so the command line help isn't overly helpful 
and it seems Tortoise doesn't have this functionality with its Create Patch 
menu item or related options. Sorry for the lack of context in my latest 
upload, but it does have two important changes:

1. noexcept now tracks its full source range, not just the start location of 
the noexcept token,
2. now, with actual tests.


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman updated this revision to Diff 57798.
aaron.ballman added a comment.

For noexcept specifications without an expression, now tracking the full source 
range. Also, added tests.


http://reviews.llvm.org/D20428

Files:
  include/clang/AST/Decl.h
  include/clang/AST/TypeLoc.h
  lib/AST/Decl.cpp
  lib/Parse/ParseDeclCXX.cpp
  lib/Sema/SemaType.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  unittests/AST/SourceLocationTest.cpp

Index: unittests/AST/SourceLocationTest.cpp
===
--- unittests/AST/SourceLocationTest.cpp
+++ unittests/AST/SourceLocationTest.cpp
@@ -580,5 +580,44 @@
   Language::Lang_CXX11));
 }
 
+class ExceptionSpecRangeVerifier : public RangeVerifier {
+protected:
+  SourceRange getRange(const TypeLoc ) override {
+auto T =
+  Node.getUnqualifiedLoc().castAs();
+assert(!T.isNull());
+return T.getExceptionSpecRange();
+  }
+};
+
+TEST(FunctionDecl, ExceptionSpecifications) {
+  ExceptionSpecRangeVerifier Verifier;
+
+  Verifier.expectRange(1, 10, 1, 16);
+  EXPECT_TRUE(Verifier.match("void f() throw();\n", loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 34);
+  EXPECT_TRUE(Verifier.match("void f() throw(void(void) throw());\n",
+ loc(functionType(;
+
+  Verifier.expectRange(1, 10, 1, 19);
+  std::vector Args;
+  Args.push_back("-fms-extensions");
+  EXPECT_TRUE(Verifier.match("void f() throw(...);\n", loc(functionType()),
+ Args, Language::Lang_CXX));
+
+  Verifier.expectRange(1, 10, 1, 17);
+  EXPECT_TRUE(Verifier.match("void f() noexcept;\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 24);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(false);\n", loc(functionType()),
+ Language::Lang_CXX11));
+
+  Verifier.expectRange(1, 10, 1, 32);
+  EXPECT_TRUE(Verifier.match("void f() noexcept(noexcept(1+1));\n",
+ loc(functionType()), Language::Lang_CXX11));
+}
+
 } // end namespace ast_matchers
 } // end namespace clang
Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -572,6 +572,7 @@
   Record.AddSourceLocation(TL.getLocalRangeBegin());
   Record.AddSourceLocation(TL.getLParenLoc());
   Record.AddSourceLocation(TL.getRParenLoc());
+  Record.AddSourceRange(TL.getExceptionSpecRange());
   Record.AddSourceLocation(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i)
 Record.AddDeclRef(TL.getParam(i));
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5812,6 +5812,8 @@
   TL.setLocalRangeBegin(ReadSourceLocation(Record, Idx));
   TL.setLParenLoc(ReadSourceLocation(Record, Idx));
   TL.setRParenLoc(ReadSourceLocation(Record, Idx));
+  TL.setExceptionSpecRange(SourceRange(ReadSourceLocation(Record, Idx),
+   ReadSourceLocation(Record, Idx)));
   TL.setLocalRangeEnd(ReadSourceLocation(Record, Idx));
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i) {
 TL.setParam(i, ReadDeclAs(Record, Idx));
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4919,6 +4919,7 @@
   NewTL.setLocalRangeBegin(TL.getLocalRangeBegin());
   NewTL.setLParenLoc(TL.getLParenLoc());
   NewTL.setRParenLoc(TL.getRParenLoc());
+  NewTL.setExceptionSpecRange(TL.getExceptionSpecRange());
   NewTL.setLocalRangeEnd(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = NewTL.getNumParams(); i != e; ++i)
 NewTL.setParam(i, ParamDecls[i]);
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5065,7 +5065,7 @@
 ParmVarDecl *Param = cast(FTI.Params[i].Param);
 TL.setParam(tpi++, Param);
   }
-  // FIXME: exception specs
+  TL.setExceptionSpecRange(FTI.getExceptionSpecRange());
 }
 void VisitParenTypeLoc(ParenTypeLoc TL) {
   assert(Chunk.Kind == DeclaratorChunk::Paren);
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -3400,7 +3400,8 @@
 
   // If we already had a dynamic specification, parse the noexcept for,
   // recovery, but emit a diagnostic and don't store the results.
-  SourceRange NoexceptRange;
+  SourceRange NoexceptRange(Tok.getLocation(),
+Tok.getEndLoc().getLocWithOffset(-1));
   ExceptionSpecificationType NoexceptType = EST_None;
 
   SourceLocation KeywordLoc 

Re: [PATCH] D20428: Tracking exception specification source locations

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

In http://reviews.llvm.org/D20428#434242, @aaron.ballman wrote:

> In http://reviews.llvm.org/D20428#434238, @alexfh wrote:
>
> > Full context diff, please.
>
>
> Pardon my complete ignorance, but how? I generated the diff from svn the 
> usual way, so I assume I've missed some step.


There are at least two ways to do this:

1. both `git diff` and `svn diff` can be convinced to produce diffs with full 
context: 
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
2. I find arcanist a very useful alternative to using web-interface for posting 
diffs: 
http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-command-line


http://reviews.llvm.org/D20428



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

Re: [PATCH] D20428: Tracking exception specification source locations

[Alexander Kornienko via cfe-commits]

alexfh added a comment.

Full context diff, please.

> I'm not certain of the best way to test this functionality in isolation;


Same way other locations/ranges are tested in 
unittests/AST/SourceLocationTest.cpp?


http://reviews.llvm.org/D20428



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

[PATCH] D20428: Tracking exception specification source locations

[Aaron Ballman via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added a reviewer: rsmith.
aaron.ballman added subscribers: cfe-commits, hintonda, alexfh.

We do not currently track the source locations for exception specifications 
such that their source range can be queried through the AST. This leads to 
trying to write more complex code to determine the source range for uses like 
FixItHints (see D18575 for an example). In addition to use within tools like 
clang-tidy, I think this information may become more important to track as 
exception specifications become more integrated into the type system.

One thing this patch is missing is a test case. I'm not certain of the best way 
to test this functionality in isolation; suggestions welcome.

http://reviews.llvm.org/D20428

Files:
  include/clang/AST/Decl.h
  include/clang/AST/TypeLoc.h
  lib/AST/Decl.cpp
  lib/Sema/SemaType.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp

Index: lib/Serialization/ASTWriter.cpp
===
--- lib/Serialization/ASTWriter.cpp
+++ lib/Serialization/ASTWriter.cpp
@@ -572,6 +572,7 @@
   Record.AddSourceLocation(TL.getLocalRangeBegin());
   Record.AddSourceLocation(TL.getLParenLoc());
   Record.AddSourceLocation(TL.getRParenLoc());
+  Record.AddSourceRange(TL.getExceptionSpecRange());
   Record.AddSourceLocation(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i)
 Record.AddDeclRef(TL.getParam(i));
Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -5812,6 +5812,8 @@
   TL.setLocalRangeBegin(ReadSourceLocation(Record, Idx));
   TL.setLParenLoc(ReadSourceLocation(Record, Idx));
   TL.setRParenLoc(ReadSourceLocation(Record, Idx));
+  TL.setExceptionSpecRange(SourceRange(ReadSourceLocation(Record, Idx),
+   ReadSourceLocation(Record, Idx)));
   TL.setLocalRangeEnd(ReadSourceLocation(Record, Idx));
   for (unsigned i = 0, e = TL.getNumParams(); i != e; ++i) {
 TL.setParam(i, ReadDeclAs(Record, Idx));
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -4919,6 +4919,7 @@
   NewTL.setLocalRangeBegin(TL.getLocalRangeBegin());
   NewTL.setLParenLoc(TL.getLParenLoc());
   NewTL.setRParenLoc(TL.getRParenLoc());
+  NewTL.setExceptionSpecRange(TL.getExceptionSpecRange());
   NewTL.setLocalRangeEnd(TL.getLocalRangeEnd());
   for (unsigned i = 0, e = NewTL.getNumParams(); i != e; ++i)
 NewTL.setParam(i, ParamDecls[i]);
Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -5065,7 +5065,7 @@
 ParmVarDecl *Param = cast(FTI.Params[i].Param);
 TL.setParam(tpi++, Param);
   }
-  // FIXME: exception specs
+  TL.setExceptionSpecRange(FTI.getExceptionSpecRange());
 }
 void VisitParenTypeLoc(ParenTypeLoc TL) {
   assert(Chunk.Kind == DeclaratorChunk::Paren);
Index: lib/AST/Decl.cpp
===
--- lib/AST/Decl.cpp
+++ lib/AST/Decl.cpp
@@ -2935,6 +2935,18 @@
   return RTRange;
 }
 
+SourceRange FunctionDecl::getExceptionSpecSourceRange() const {
+  const TypeSourceInfo *TSI = getTypeSourceInfo();
+  if (!TSI)
+return SourceRange();
+  FunctionTypeLoc FTL =
+TSI->getTypeLoc().IgnoreParens().getAs();
+  if (!FTL)
+return SourceRange();
+
+  return FTL.getExceptionSpecRange();
+}
+
 const Attr *FunctionDecl::getUnusedResultAttr() const {
   QualType RetType = getReturnType();
   if (RetType->isRecordType()) {
Index: include/clang/AST/TypeLoc.h
===
--- include/clang/AST/TypeLoc.h
+++ include/clang/AST/TypeLoc.h
@@ -1248,6 +1248,7 @@
   SourceLocation LocalRangeBegin;
   SourceLocation LParenLoc;
   SourceLocation RParenLoc;
+  SourceRange ExceptionSpecRange;
   SourceLocation LocalRangeEnd;
 };
 
@@ -1289,6 +1290,13 @@
 return SourceRange(getLParenLoc(), getRParenLoc());
   }
 
+  SourceRange getExceptionSpecRange() const {
+return this->getLocalData()->ExceptionSpecRange;
+  }
+  void setExceptionSpecRange(SourceRange R) {
+this->getLocalData()->ExceptionSpecRange = R;
+  }
+
   ArrayRef getParams() const {
 return llvm::makeArrayRef(getParmArray(), getNumParams());
   }
@@ -1318,6 +1326,7 @@
 setLocalRangeBegin(Loc);
 setLParenLoc(Loc);
 setRParenLoc(Loc);
+setExceptionSpecRange(SourceRange(Loc));
 setLocalRangeEnd(Loc);
 for (unsigned i = 0, e = getNumParams(); i != e; ++i)
   setParam(i, nullptr);
Index: include/clang/AST/Decl.h
===
--- include/clang/AST/Decl.h
+++