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&id=84127#toc
Repository:
rL LLVM
https://re
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.l
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
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
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
___
c
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/li
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/Se
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
__
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/m
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/T
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:
> Ric
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
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:
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.
https://reviews.llvm.org/D20428
__
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/Par
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 Type
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
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@l
On Fri, Jun 24, 2016 at 10:30 AM, Aaron Ballman wrote:
> 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
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 `TypeLoc
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 `TypeLoc
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
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);
>
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
undecl
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)
+re
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 remov
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 `CreateTypeSou
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
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;
+ Sourc
On Fri, May 27, 2016 at 12:19 PM, don hinton wrote:
> 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 a
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/Se
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;
+
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;
+ Sourc
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;
+ Sourc
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/Parse
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(),
+
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
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(),
+
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
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
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 catc
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.c
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 mail
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, b
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/D
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
aaron.ballman added a comment.
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.
> > I'm not certain of the best way to test this function
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
___
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
tryi
49 matches
Mail list logo
