[Lldb-commits] [PATCH] D143347: [lldb][DWARF] Infer no_unique_address attribute

balazske added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:3896 + if (D->hasAttrs()) +ToField->setAttrs(D->getAttrs()); ToField->setAccess(D->getAccess()); The import of attributes is handled in function `ASTImporter::Import(Decl*)`. This

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

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

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

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

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

balazske added inline comments. Comment at: lldb/include/lldb/Symbol/ClangASTImporter.h:249 +/// it at the end of the scope. Supports being used multiple times on the +/// same Minion instance in nested scopes. +class CxxModuleScope { teemperor wrote:

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

balazske added inline comments. Comment at: clang/unittests/AST/ASTImporterTest.cpp:583 + // Returns a ImporterConstructor that constructs this class. + static ASTImporterOptionSpecificTestBase::ImporterConstructor + getConstructor() { balazske wrote: > Is it

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

balazske added inline comments. Comment at: lldb/source/Symbol/StdModuleHandler.cpp:245 +new_class_template->AddSpecialization(found_decl, InsertPos); +new_class_template->dumpColor(); +if (new_class_template->isOutOfLine()) This dump is not needed?

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

balazske added inline comments. Comment at: lldb/source/Symbol/StdModuleHandler.cpp:220 +case TemplateArgument::Type: { + QualType our_type = m_importer.Import(arg.getAsType()); + imported_args.push_back(TemplateArgument(our_type)); teemperor wrote:

[Lldb-commits] [PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

balazske added inline comments. Comment at: clang/lib/AST/ASTImporter.cpp:7738 +Expected ASTImporter::ImportInternal(Decl *FromD) { + // Import the declaration. + ASTNodeImporter Importer(*this); Comment can be better: Import it using ASTNodeImporter. ===

[Lldb-commits] [PATCH] D59537: Instantiate 'std' templates explicitly in the expression evaluator

balazske added inline comments. Comment at: lldb/source/Symbol/StdModuleHandler.cpp:220 +case TemplateArgument::Type: { + QualType our_type = m_importer.Import(arg.getAsType()); + imported_args.push_back(TemplateArgument(our_type)); For long-term (b

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

balazske added a comment. By the way if the `Import` of the strategy uses recursive import of other things this can cause same problems as it was in `ASTImporter` before the `GetImportedOrCreateDecl` was introduced. So this should be avoided or something similar to `GetImportedOrCreateDecl` mus

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

balazske added a comment. Another observation: The `Import` function of the strategy now has no way to return an error. An even better version of it would be to include the possibility of import error (with `ImportError`, or other error type). Or the "PreImport" function could indicate if the D

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

balazske added a comment. It looks better now. One "problem" is that now there is the strategy and there is the `Imported` function. It would be possible to unify these into a strategy that contains maybe a "import" and a "post-import" callback. (But this requires big changes in the `ASTImporte

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a shim to the ASTImporter

balazske added a comment. The replace of `Import` calls may work if the internal state of the `ASTImporter` is updated correctly from the 'shim' object (especially if not every Decl is handled by the shim). For this to work some of the state of `ASTImporter` that was intended to be internal mus

[Lldb-commits] [PATCH] D59485: [ASTImporter] Allow adding a shim to the ASTImporter

balazske added a comment. I do not know if "Shim" is the correct name for this, maybe "Strategy" is better? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D59485/new/ https://reviews.llvm.org/D59485 ___ lldb-commits m