aaron.ballman added a comment.

There is no information in this patch as to why the changes are needed or 
useful; please use a more descriptive summary next time.



================
Comment at: clang/unittests/Interpreter/InterpreterTest.cpp:205-207
+  // FIXME: We cannot yet handle delayed template parsing. If we run with
+  // -fdelayed-template-parsing we try adding the newly created decl to the
+  // active PTU which causes an assert.
----------------
FWIW, Intel's internal testing is hitting crashes with this test. With 
`-fno-delayed-template-parsing` enabled, we get a failed assertion:
```
Assertion failed: OffsetBytes <= AllocationSize && "Offset out of bounds!", 
file d:\iusers\ayrivera\dev-xmain-web\llvm\l
lvm\lib\executionengine\runtimedyld\RuntimeDyldImpl.h, line 90
```
and when we enable the delayed template parsing, we get a different assertion:
```
Assertion failed: DC->getLexicalParent() == CurContext && "The next DeclContext 
should be lexically contained in the current one.", file 
D:/iusers/ayrivera/dev-xmain-web/llvm/clang/lib/Sema/SemaDecl.cpp, line 1305
```

I think this FIXME either needs to be addressed or the patch reverted until 
this is addressed.


================
Comment at: clang/unittests/Interpreter/InterpreterTest.cpp:237-240
+  std::string MangledName = MangleName(TmpltSpec);
+  typedef int (*TemplateSpecFn)(void *);
+  auto fn = (TemplateSpecFn)cantFail(Interp->getSymbolAddress(MangledName));
+  EXPECT_EQ(42, fn(NewA));
----------------
This test is broken for some of our internal build bots at Intel. I think 
something suspicious is going on here, but I'm not certain of the intent behind 
the test, so I'm not certain the best way to fix it. The behavior of the test 
is that on an x86 Windows machine, sometimes this particular test fails:
```
[ RUN ] IncrementalProcessing.InstantiateTemplate^M
unknown file: error: SEH exception with code 0x3221225477 thrown in the test 
body.^M
[ FAILED ] IncrementalProcessing.InstantiateTemplate (35 ms)^M
```
but it's not a consistent failure (seems to happen about one out of every three 
runs).

`callme` is a templated member function of `B` but here we're trying to call it 
like it's a free function. That's... not good. We could either make `callme` a 
`static` member function so that it can be called in this manner, or we could 
try to come up with a magic incantation to call it as a PMF.

Can you investigate, @v.g.vassilev? If it is going to take considerable time to 
resolve, it might be worth reverting temporarily. Thanks!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112663/new/

https://reviews.llvm.org/D112663

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

Reply via email to