[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2021-01-05 Thread Balázs Kéri via Phabricator via cfe-commits 
balazske added a comment.

There are still problems related to import type argument default values: If 
there are forward declarations of the same template, the "inheritedness" of 
these arguments (in AST) is not set correctly and the default arguments can 
appear at more places instead of at only one (that the others reference to).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2021-01-05 Thread Balázs Kéri via Phabricator via cfe-commits 
balazske added a comment.

The problem is reproduced and fixed in D94067 
. It is caused by import of default template 
arguments indirectly, because that import causes lot of other things to be 
imported. And the import of default arguments happens in a incomplete state, 
when the "templated" CXXRecordDecl does exist but has no described template set 
yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-12-02 Thread Raphael Isemann via Phabricator via cfe-commits 
teemperor added a comment.

In D92103#2428139 , @martong wrote:

> Hey Raphael, thanks for looking into the CTU crash!
>
> I also had a look and I could reproduce that on Linux Ubuntu 18.04 with GCC 
> 7. I think the discrepancy stems from GCC's libstdc++ and MacOS's libc++ 
> implementation differences of `basic_streambuf`. This is the CXXRecordDecl 
> which does not need an injected class name type (so there is the assert). 
> However, the situation is more complex because there is a 5 long redecl chain 
> (streambuf is forward declared in many places).
>
> Anyway, I don't think that this patch itself caused the crash, it just 
> revealed some other issues that are related to the possibly flawed import 
> logic of injected class (name) types. I am adding another college @balazske 
> as a subscriber because he started to investigate the crash deeper.

Quick note: I should have mention that I actually tried reproducing this issue 
with libstdc++/GCC 10.2.0 on Arch Linux.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-12-02 Thread Gabor Marton via Phabricator via cfe-commits 
martong added a subscriber: balazske.
martong added a comment.

Hey Raphael, thanks for looking into the CTU crash!

I also had a look and I could reproduce that on Linux Ubuntu 18.04 with GCC 7. 
I think the discrepancy stems from GCC's libstdc++ and MacOS's libc++ 
implementation differences of `basic_streambuf`. This is the CXXRecordDecl 
which does not need an injected class name type (so there is the assert). 
However, the situation is more complex because there is a 5 long redecl chain 
(streambuf is forward declared in many places).

Anyway, I don't think that this patch itself caused the crash, it just revealed 
some other issues that are related to the possibly flawed import logic of 
injected class (name) types. I am adding another college @balazske as a 
subscriber because he started to investigate the crash deeper.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-12-01 Thread Raphael Isemann via Phabricator via cfe-commits 
teemperor added a comment.

@gamesh411 I recreated the setup you listed (thanks for that btw) but for me 
this works just fine. I assume the crash happens in a class template from one 
of the external libraries. It probably works for me because I don't have the 
same library version as you have, but just from the backtrace it's hard to know 
where the error could come from.

I'm not sure if the XTU analyzer has a way to create reproducers (maybe 
@martong knows), but if you could apply the patch below, recompile/run the 
analyzer and post the output that would help a lot with figuring out what code 
on your system is causing the crash:

  diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
  index 67ee8c0956d6..a471501aa71e 100644
  --- a/clang/lib/AST/ASTContext.cpp
  +++ b/clang/lib/AST/ASTContext.cpp
  @@ -4408,6 +4408,9 @@ static bool NeedsInjectedClassNameType(const RecordDecl 
*D) {
   /// injected class name type for the specified templated declaration.
   QualType ASTContext::getInjectedClassNameType(CXXRecordDecl *Decl,
 QualType TST) const {
  +  if (!NeedsInjectedClassNameType(Decl)) {
  +Decl->dumpColor();
  +  }
 assert(NeedsInjectedClassNameType(Decl));
 if (Decl->TypeForDecl) {
   assert(isa(Decl->TypeForDecl));

The output from this code is probably colorized in your terminal and looks a 
bit like this:

  |-ClassTemplateDecl 0x7ff94c04a0a0  line:1:36 A
  | |-TemplateTypeParmDecl 0x7ff94c049f50  col:26 referenced 
typename depth 0 index 0 T
  | |-CXXRecordDecl 0x7ff94c04a010  line:1:36 struct A 
definition
  | | |-DefinitionData empty standard_layout trivially_copyable 
has_user_declared_ctor can_const_default_init
  | | | |-DefaultConstructor defaulted_is_constexpr
  | | | |-CopyConstructor simple trivial has_const_param needs_implicit 
implicit_has_const_param
  | | | |-MoveConstructor exists simple trivial needs_implicit
  [...]

FWIW, I don't think the patch itself introduces this regression, but it just 
causes more of the AST to be imported (and the import of these AST nodes then 
runs into an unsupported use case). This would most likely already crash if the 
analysed source code referenced that class template in some other way.




Comment at: 
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py:25
 
-deque_type = "std::deque >"
 size_type = deque_type + "::size_type"

shafik wrote:
> Why do the default arguments not show up in the results anymore?
Because we don't show args that match the default arg in the display type 
(which is identical to what Clang does). See also rdar://59292534


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-11-30 Thread Shafik Yaghmour via Phabricator via cfe-commits 
shafik added inline comments.



Comment at: 
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py:25
 
-deque_type = "std::deque >"
 size_type = deque_type + "::size_type"

Why do the default arguments not show up in the results anymore?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-11-28 Thread Endre Fülöp via Phabricator via cfe-commits 
gamesh411 added a comment.

On bitcoin v0.18.1 , there is 
an assertion introduced by this change.
The TU that can be used for reproduction is `src/script/interpreter.cpp`.
Assertion message:

  CTU loaded AST file: /home/gamesh411/bitcoin/src/script/script.cpp

 
  clang: /home/gamesh411/llvm-project/clang/lib/AST/ASTContext.cpp:4411: 
clang::QualType 
clang::ASTContext::getInjectedClassNameType(clang::CXXRecordDecl*, 
clang::QualType) const: Assertion `NeedsInjectedC
  lassNameType(Decl)' failed.

Stacktrace:

  1.   parser at end of file   

  [310/31632]
  2.  While analyzing stack: 
  #0 Calling CountWitnessSigOps
  3.  /home/gamesh411/bitcoin/src/script/interpreter.cpp:1618:9: Error 
evaluating statement
  4.  /home/gamesh411/bitcoin/src/script/interpreter.cpp:1618:9: Error 
evaluating statement
#0 0x7f9063076451 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) 
(/home/gamesh411/clang-rwa/bin/../lib/libLLVMSupport.so.12git+0x1be451)
#1 0x7f9063073ff4 llvm::sys::RunSignalHandlers() 
(/home/gamesh411/clang-rwa/bin/../lib/libLLVMSupport.so.12git+0x1bbff4)
#2 0x7f9063074291 llvm::sys::CleanupOnSignal(unsigned long) 
(/home/gamesh411/clang-rwa/bin/../lib/libLLVMSupport.so.12git+0x1bc291)
#3 0x7f9062f7d0b8 CrashRecoverySignalHandler(int) 
(/home/gamesh411/clang-rwa/bin/../lib/libLLVMSupport.so.12git+0xc50b8)
#4 0x7f9062b05210 (/lib/x86_64-linux-gnu/libc.so.6+0x46210)
#5 0x7f9062b0518b raise 
/build/glibc-ZN95T4/glibc-2.31/signal/../sysdeps/unix/sysv/linux/raise.c:51:1
#6 0x7f9062ae4859 abort 
/build/glibc-ZN95T4/glibc-2.31/stdlib/abort.c:81:7
#7 0x7f9062ae4729 get_sysdep_segment_value 
/build/glibc-ZN95T4/glibc-2.31/intl/loadmsgcat.c:509:8
#8 0x7f9062ae4729 _nl_load_domain 
/build/glibc-ZN95T4/glibc-2.31/intl/loadmsgcat.c:970:34
#9 0x7f9062af5f36 (/lib/x86_64-linux-gnu/libc.so.6+0x36f36)
   #10 0x7f90612e5be0 
clang::ASTContext::getInjectedClassNameType(clang::CXXRecordDecl*, 
clang::QualType) const 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x1a4be0)
   #11 0x7f9061393933 
clang::ASTNodeImporter::VisitRecordDecl(clang::RecordDecl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x252933)
   #12 0x7f9061361055 clang::declvisitor::Base >::Visit(clang::Decl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.
  12git+0x220055)
   #13 0x7f906136171e clang::ASTImporter::Import(clang::Decl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x22071e)
   #14 0x7f906139cfad 
clang::ASTNodeImporter::VisitClassTemplateDecl(clang::ClassTemplateDecl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x25bfad)
   #15 0x7f9061361125 clang::declvisitor::Base >::Visit(clang::Decl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.
  12git+0x220125)
   #16 0x7f906136171e clang::ASTImporter::Import(clang::Decl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x22071e)
   #17 0x7f90613631cc llvm::Expected 
clang::ASTNodeImporter::import(clang::Decl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x2221cc)
   #18 0x7f9061370385 
clang::ASTNodeImporter::ImportDeclContext(clang::DeclContext*, bool) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x22f385)
   #19 0x7f906139013b 
clang::ASTNodeImporter::VisitNamespaceDecl(clang::NamespaceDecl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x24f13b)
   #20 0x7f90613612b5 clang::declvisitor::Base >::Visit(clang::Decl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.
  12git+0x2202b5)
   #21 0x7f906136171e clang::ASTImporter::Import(clang::Decl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x22071e)
   #22 0x7f90613631cc llvm::Expected 
clang::ASTNodeImporter::import(clang::Decl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x2221cc)
   #23 0x7f9061370385 
clang::ASTNodeImporter::ImportDeclContext(clang::DeclContext*, bool) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x22f385)
   #24 0x7f906139013b 
clang::ASTNodeImporter::VisitNamespaceDecl(clang::NamespaceDecl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.12git+0x24f13b)
   #25 0x7f90613612b5 clang::declvisitor::Base >::Visit(clang::Decl*) 
(/home/gamesh411/clang-rwa/bin/../lib/../lib/libclangAST.so.
  12git+0x2202b5)
   #26 0x7f906136171e clang::ASTImporter::Import(clang::Decl*)

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-11-26 Thread Raphael Isemann via Phabricator via cfe-commits 
teemperor added inline comments.
Herald added a subscriber: JDevlieghere.



Comment at: clang/lib/AST/ASTImporter.cpp:5161
 
-  // FIXME: Import default argument  and constraint expression.
+  // FIXME: Import constraint expression.
 

martong wrote:
> I wonder whether this FIXME is already fixed, at line 5182?
I think so. I removed the whole comment instead. Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

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

[PATCH] D92103: [ASTImporter] Import the default argument of TemplateTypeParmDecl

2020-11-26 Thread Raphael Isemann via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rG3f6c856bb5ae: [ASTImporter] Import the default argument of 
TemplateTypeParmDecl (authored by teemperor).
Herald added projects: clang, LLDB.
Herald added subscribers: lldb-commits, cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D92103?vs=307614=307891#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D92103

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp
  
lldb/test/API/commands/expression/import-std-module/deque-basic/TestDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/deque-dbg-info-content/TestDbgInfoContentDequeFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/forward_list-dbg-info-content/TestDbgInfoContentForwardListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/forward_list/TestForwardListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/list-dbg-info-content/TestDbgInfoContentListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/list/TestListFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/queue/TestQueueFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/stack/TestStackFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/unique_ptr-dbg-info-content/TestUniquePtrDbgInfoContent.py
  
lldb/test/API/commands/expression/import-std-module/unique_ptr/TestUniquePtrFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-bool/TestVectorBoolFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
  
lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py

Index: lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector/TestVectorFromStdModule.py
@@ -22,7 +22,7 @@
 
 self.runCmd("settings set target.import-std-module true")
 
-vector_type = "std::vector >"
+vector_type = "std::vector"
 size_type = vector_type + "::size_type"
 value_type = "std::__vector_base >::value_type"
 iterator = vector_type + "::iterator"
Index: lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector-of-vectors/TestVectorOfVectorsFromStdModule.py
@@ -20,13 +20,9 @@
   "// Set break point at this line.",
   lldb.SBFileSpec("main.cpp"))
 
-vector_type = "std::vector >"
-vector_of_vector_type = "std::vector<" + vector_type + \
-", std::allocator > > >"
-size_type = (
-"std::vector >, " +
-"std::allocator > >" +
-" >::size_type")
+vector_type = "std::vector"
+vector_of_vector_type = "std::vector<" + vector_type + " >"
+size_type = vector_of_vector_type + "::size_type"
 value_type = "std::__vector_base >::value_type"
 
 self.runCmd("settings set target.import-std-module true")
@@ -35,13 +31,13 @@
 "a",
 result_type=vector_of_vector_type,
 result_children=[
-ValueCheck(type="std::vector >",
+ValueCheck(type="std::vector",
children=[
ValueCheck(value='1'),
ValueCheck(value='2'),
ValueCheck(value='3'),
]),
-ValueCheck(type="std::vector >",
+ValueCheck(type="std::vector",
children=[
ValueCheck(value='3'),
ValueCheck(value='2'),
Index: lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
===
--- lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
+++ lldb/test/API/commands/expression/import-std-module/vector-dbg-info-content/TestDbgInfoContentVectorFromStdModule.py
@@ -23,7 +23,7 @@