On 25/11/14 00:52, Richard Smith wrote:
+ } + else if (!isUnresolvedExceptionSpec(FPT->getExceptionSpecType()) &&
Per the LLVM coding style, put the close brace and the 'else' on the
same line.
+ isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType())) { + //
Happens in cases where module A contains only a fwd decl and module B
+ // contains the definition.
There are other conditions here; I don't think this comment is
particularly helpful. Just remove it?
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo(); +
while (PrevFD) { + Reader.Context.adjustExceptionSpec(PrevFD,
EPI.ExceptionSpec); + PrevFD = PrevFD->getPreviousDecl();
Please use a separate variable for this loop rather than reusing
PrevFD. It's only by coincidence that this code is at the end of the
function; it shouldn't be changing function-scope state like this.
All good points, thanks!
It seems like it should be possible to produce a testcase for this.
You'd need something like:
A.h:
struct A { A(); } b{}, c(b); // computed exception spec for copy ctor
and move ctor
B.h:
struct A { A(); } a{}; // EST_Unevaluated for copy ctor and move ctor
... then import A and B, and do something that assumes that every
declaration has an exception specification if any declaration does.
Thanks for the pointer. I managed to reproduce the behavior, i.e
unevaluated exception spec as a first/canonical redecl (see the new
attached patch). However this test doesn't trigger the original issue
(and thus not testing anything :( )
http://llvm.org/bugs/show_bug.cgi?id=21687
There the setup is more difficult, because I need to generate a
unevaluated exception spec dtor as a first redecl and go through
clang::FunctionProtoType::isNothrow to get the assertion. To make things
worse, this base dtor must be emitted as an alias. Could you help me out?
Vassil
On Mon, Nov 24, 2014 at 11:09 AM, Vassil Vassilev <[email protected]
<mailto:[email protected]>> wrote:
Sorry for the delay. Attaching the new version.
Vassil
On 14/11/14 02:47, Richard Smith wrote:
+ }
+ else { // FPT->getExceptionSpecType() is resolved and the
other is not
You're not checking for this condition; the code here is getting
run even if both or neither are unresolved.
The patch needs rebasing (we have a new helper function in
ASTContext to update the exception specification of a
declaration), but looks like the right direction.
On Thu, Nov 6, 2014 at 4:23 AM, Vassil Vassilev
<[email protected]
<mailto:[email protected]>> wrote:
Hi Richard,
I am attaching the patch we discussed at the dev meeting.
Still haven't found small reproducer...
The issue appears to stem from the fact that module A
contains only a forward declaration of a function and it
exception spec cannot be computed. In module B is the
definition with computed exception spec, which gets
deserialized after the one in module A. This patch teaches
the ASTDeclReader to update all the exception specs of the
previous decls to the current one.
Could you review, please?
Many thanks,
Vassil
_______________________________________________
cfe-commits mailing list
[email protected] <mailto:[email protected]>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
--------------------------------------------
Q: Why is this email five sentences or less?
A: http://five.sentenc.es
Index: lib/Serialization/ASTReaderDecl.cpp
===================================================================
--- lib/Serialization/ASTReaderDecl.cpp
+++ lib/Serialization/ASTReaderDecl.cpp
@@ -2779,11 +2779,20 @@ void ASTDeclReader::attachPreviousDeclImpl(ASTReader
&Reader,
// specification now.
auto *FPT = FD->getType()->getAs<FunctionProtoType>();
auto *PrevFPT = PrevFD->getType()->getAs<FunctionProtoType>();
- if (FPT && PrevFPT &&
- isUnresolvedExceptionSpec(FPT->getExceptionSpecType()) &&
- !isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType())) {
- Reader.Context.adjustExceptionSpec(
+ if (FPT && PrevFPT) {
+ if (isUnresolvedExceptionSpec(FPT->getExceptionSpecType()) &&
+ !isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType())) {
+ Reader.Context.adjustExceptionSpec(
FD, PrevFPT->getExtProtoInfo().ExceptionSpec);
+ } else if (!isUnresolvedExceptionSpec(FPT->getExceptionSpecType()) &&
+ isUnresolvedExceptionSpec(PrevFPT->getExceptionSpecType())) {
+ FunctionProtoType::ExtProtoInfo EPI = FPT->getExtProtoInfo();
+ FunctionDecl* PrevFDToUpdate = PrevFD;
+ while (PrevFDToUpdate) {
+ Reader.Context.adjustExceptionSpec(PrevFDToUpdate, EPI.ExceptionSpec);
+ PrevFDToUpdate = PrevFDToUpdate->getPreviousDecl();
+ }
+ }
}
}
}
Index: test/Modules/Inputs/PR21687/A.h
new file mode 100644
===================================================================
--- /dev/null
+++ test/Modules/Inputs/PR21687/A.h
@@ -0,0 +1,4 @@
+struct A {
+ A(){}
+ virtual ~A() {}
+} b{}, c(b); // computed exception spec for copy ctor and move ctor
Index: test/Modules/Inputs/PR21687/B.h
new file mode 100644
===================================================================
--- /dev/null
+++ test/Modules/Inputs/PR21687/B.h
@@ -0,0 +1,4 @@
+struct A {
+ A(){}
+ virtual ~A() {}
+} a{}; // EST_Unevaluated for copy ctor and move ctor
Index: test/Modules/Inputs/PR21687/module.modulemap
new file mode 100644
===================================================================
--- /dev/null
+++ test/Modules/Inputs/PR21687/module.modulemap
@@ -0,0 +1,10 @@
+module modA {
+ header "A.h"
+ export *
+}
+
+module modB {
+ header "B.h"
+ export *
+}
+
Index: test/Modules/pr21687.cpp
new file mode 100644
===================================================================
--- /dev/null
+++ test/Modules/pr21687.cpp
@@ -0,0 +1,10 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -x c++ -std=c++11 %s
+
+#include "Inputs/PR21687/B.h"
+A tmp;
+#include "Inputs/PR21687/A.h"
+
+int main() {
+ A tmp1;
+}
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
