Index: lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp =================================================================== --- lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp +++ lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp @@ -158,12 +158,23 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer { public: ClangDiagnosticManagerAdapter(DiagnosticOptions &opts) { + // Diagnostic printer for real SourceLocations. DiagnosticOptions *options = new DiagnosticOptions(opts); options->ShowPresumedLoc = true; options->ShowLevel = false; m_os = std::make_shared(m_output); m_passthrough = std::make_shared(*m_os, options); + + // Diagnostic printer for LLDB-generated dummy locations that have + // artificial line/column numbers and artificial source code. + DiagnosticOptions *dummy_options = new DiagnosticOptions(opts); + dummy_options->ShowLine = false; + dummy_options->ShowColumn = false; + dummy_options->ShowSourceRanges = false; + dummy_options->ShowCarets = false; + m_dummy_passthrough = + std::make_shared(*m_os, dummy_options); } void ResetManager(DiagnosticManager *manager = nullptr) { @@ -181,6 +192,16 @@ return clang_diag; } + /// Returns true if the SourceLocation is from a LLDB-generated dummy file. + bool IsDummySourceLocation(SourceLocation loc) const { + if (!m_ts) + return false; + if (loc.isInvalid()) + return false; + FileID file = m_ts->getASTContext().getSourceManager().getFileID(loc); + return m_ts->IsDummyFileID(file); + } + void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel, const clang::Diagnostic &Info) override { if (!m_manager) { @@ -205,7 +226,13 @@ // Render diagnostic message to m_output. m_output.clear(); - m_passthrough->HandleDiagnostic(DiagLevel, Info); + // Determine which is the right diagnostic printer for this diagnostic. + // For dummy files we use special diagnostic settings to hide confusing + // information from the user (wrong line/columns and generated source code). + if (IsDummySourceLocation(Info.getLocation())) + m_dummy_passthrough->HandleDiagnostic(DiagLevel, Info); + else + m_passthrough->HandleDiagnostic(DiagLevel, Info); m_os->flush(); lldb_private::DiagnosticSeverity severity; @@ -266,13 +293,23 @@ void BeginSourceFile(const LangOptions &LO, const Preprocessor *PP) override { m_passthrough->BeginSourceFile(LO, PP); + m_dummy_passthrough->BeginSourceFile(LO, PP); + } + + void EndSourceFile() override { + m_passthrough->EndSourceFile(); + m_dummy_passthrough->EndSourceFile(); } - void EndSourceFile() override { m_passthrough->EndSourceFile(); } + void SetTypeSystem(TypeSystemClang *ts) { m_ts = ts; } private: + TypeSystemClang *m_ts = nullptr; DiagnosticManager *m_manager = nullptr; + /// Diagnostic printer for real SourceLocations. std::shared_ptr m_passthrough; + /// Diagnostic printer for LLDB-generated dummy locations. + std::shared_ptr m_dummy_passthrough; /// Output stream of m_passthrough. std::shared_ptr m_os; /// Output string filled by m_os. @@ -650,6 +687,7 @@ m_ast_context = std::make_unique( "Expression ASTContext for '" + m_filename + "'", ast_context); + diag_mgr->SetTypeSystem(m_ast_context.get()); std::string module_name("$__lldb_module"); Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h @@ -223,6 +223,11 @@ /// contexts where the usual C++ rules require a type to be complete (base /// class, member, etc.). void CompleteType(lldb_private::CompilerType type); + + /// Wrapper for TypeSystemClang::GetLocForDecl that automatically returns an + /// invalid SourceLocation in case source locations are disabled. + clang::SourceLocation GetLocForDecl(const DWARFDIE &die, + const lldb_private::Declaration &decl); }; /// Parsed form of all attributes that are relevant for type reconstruction. Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp @@ -791,8 +791,8 @@ clang_type = m_ast.CreateEnumerationType( attrs.name.GetCString(), GetClangDeclContextContainingDIE(die, nullptr), - GetOwningClangModule(die), attrs.decl, enumerator_clang_type, - attrs.is_scoped_enum); + GetOwningClangModule(die), GetLocForDecl(die, attrs.decl), + enumerator_clang_type, attrs.is_scoped_enum); } else { enumerator_clang_type = m_ast.GetEnumerationIntegerType(clang_type); } @@ -1189,6 +1189,9 @@ : containing_decl_ctx, GetOwningClangModule(die), name, clang_type, attrs.storage, attrs.is_inline); + const clang::SourceLocation loc = GetLocForDecl(die, attrs.decl); + function_decl->setLocation(loc); + function_decl->setRangeEnd(loc); if (has_template_params) { TypeSystemClang::TemplateParameterInfos template_param_infos; @@ -1198,10 +1201,12 @@ : containing_decl_ctx, GetOwningClangModule(die), attrs.name.GetCString(), clang_type, attrs.storage, attrs.is_inline); + template_function_decl->setLocation(loc); clang::FunctionTemplateDecl *func_template_decl = m_ast.CreateFunctionTemplateDecl( containing_decl_ctx, GetOwningClangModule(die), template_function_decl, name, template_param_infos); + func_template_decl->setLocation(loc); m_ast.CreateFunctionTemplateSpecializationInfo( template_function_decl, func_template_decl, template_param_infos); } @@ -1340,6 +1345,14 @@ m_ast.GetMetadata(td)->SetIsForcefullyCompleted(); } +clang::SourceLocation +DWARFASTParserClang::GetLocForDecl(const DWARFDIE &die, + const Declaration &decl) { + if (!die.GetCU()->GetSymbolFileDWARF().UseSourceLocations()) + return clang::SourceLocation(); + return m_ast.GetLocForDecl(decl); +} + TypeSP DWARFASTParserClang::UpdateSymbolContextScopeForType( const SymbolContext &sc, const DWARFDIE &die, TypeSP type_sp) { if (!type_sp) @@ -1602,6 +1615,7 @@ m_ast.CreateClassTemplateSpecializationDecl( decl_ctx, GetOwningClangModule(die), class_template_decl, tag_decl_kind, template_param_infos); + class_specialization_decl->setLocation(GetLocForDecl(die, attrs.decl)); clang_type = m_ast.CreateClassTemplateSpecializationType( class_specialization_decl); clang_type_was_created = true; @@ -1616,7 +1630,7 @@ clang_type = m_ast.CreateRecordType( decl_ctx, GetOwningClangModule(die), attrs.accessibility, attrs.name.GetCString(), tag_decl_kind, attrs.class_language, - &metadata, attrs.exports_symbols); + &metadata, attrs.exports_symbols, GetLocForDecl(die, attrs.decl)); } } @@ -2219,7 +2233,8 @@ if (name && name[0] && got_value) { m_ast.AddEnumerationValueToEnumerationType( - clang_type, decl, name, enum_value, enumerator_byte_size * 8); + clang_type, GetLocForDecl(die, decl), name, enum_value, + enumerator_byte_size * 8); ++enumerators_added; } } Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -289,6 +289,8 @@ std::vector> ParseCallEdgesInFunction(UserID func_id) override; + bool UseSourceLocations() const; + void Dump(lldb_private::Stream &s) override; void DumpClangAST(lldb_private::Stream &s) override; Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -140,6 +140,11 @@ return m_collection_sp->GetPropertyAtIndexAsBoolean( nullptr, ePropertyIgnoreIndexes, false); } + + bool UseSourceLocations() const { + return m_collection_sp->GetPropertyAtIndexAsBoolean( + nullptr, ePropertyUseSourceLocations, false); + } }; typedef std::shared_ptr SymbolFileDWARFPropertiesSP; @@ -3855,6 +3860,10 @@ return {}; } +bool SymbolFileDWARF::UseSourceLocations() const { + return GetGlobalPluginProperties()->UseSourceLocations(); +} + // PluginInterface protocol ConstString SymbolFileDWARF::GetPluginName() { return GetPluginNameStatic(); } Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td @@ -5,4 +5,8 @@ Global, DefaultFalse, Desc<"Ignore indexes present in the object files and always index DWARF manually.">; + def UseSourceLocations: Property<"use-source-locations", "Boolean">, + Global, + DefaultTrue, + Desc<"Generate source locations for declarations to improve error diagnostics.">; } Index: lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp @@ -1106,8 +1106,8 @@ Declaration declaration; CompilerType enum_ct = m_clang.CreateEnumerationType( - uname.c_str(), decl_context, OptionalClangModuleID(), declaration, - ToCompilerType(underlying_type), er.isScoped()); + uname.c_str(), decl_context, OptionalClangModuleID(), + clang::SourceLocation(), ToCompilerType(underlying_type), er.isScoped()); TypeSystemClang::StartTagDeclarationDefinition(enum_ct); TypeSystemClang::SetHasExternalStorage(enum_ct.GetOpaqueQualType(), true); Index: lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp +++ lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp @@ -271,7 +271,8 @@ llvm::StringRef name = DropNameScope(enumerator.getName()); m_ast_builder.clang().AddEnumerationValueToEnumerationType( - m_derived_ct, decl, name.str().c_str(), enumerator.Value); + m_derived_ct, clang::SourceLocation(), name.str().c_str(), + enumerator.Value); return Error::success(); } Index: lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp +++ lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp @@ -497,9 +497,9 @@ // Class). Set it false for now. bool isScoped = false; - ast_enum = m_ast.CreateEnumerationType(name.c_str(), decl_context, - OptionalClangModuleID(), decl, - builtin_type, isScoped); + ast_enum = m_ast.CreateEnumerationType( + name.c_str(), decl_context, OptionalClangModuleID(), + clang::SourceLocation(), builtin_type, isScoped); auto enum_decl = TypeSystemClang::GetAsEnumDecl(ast_enum); assert(enum_decl); @@ -1159,7 +1159,8 @@ uint32_t byte_size = m_ast.getASTContext().getTypeSize( ClangUtil::GetQualType(underlying_type)); auto enum_constant_decl = m_ast.AddEnumerationValueToEnumerationType( - enum_type, decl, name.c_str(), raw_value, byte_size * 8); + enum_type, clang::SourceLocation(), name.c_str(), raw_value, + byte_size * 8); if (!enum_constant_decl) return false; Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h =================================================================== --- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h +++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h @@ -301,6 +301,17 @@ &type_fields, bool packed = false); + /// Returns true iff the given FileID was created for a dummy file by + /// GetLocForDecl. + bool IsDummyFileID(clang::FileID fid); + + /// Returns the SourceLocation of the given Declaration. + /// + /// \return A SourceLocation that points to a dummy file with the + /// same file path as the given declaration. If the declaration + /// is invalid the returned SourceLocation is also invalid. + clang::SourceLocation GetLocForDecl(const Declaration &decl); + static bool IsOperator(llvm::StringRef name, clang::OverloadedOperatorKind &op_kind); @@ -321,13 +332,12 @@ bool is_framework = false, bool is_explicit = false); - CompilerType CreateRecordType(clang::DeclContext *decl_ctx, - OptionalClangModuleID owning_module, - lldb::AccessType access_type, - llvm::StringRef name, int kind, - lldb::LanguageType language, - ClangASTMetadata *metadata = nullptr, - bool exports_symbols = false); + CompilerType CreateRecordType( + clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, + lldb::AccessType access_type, llvm::StringRef name, int kind, + lldb::LanguageType language, ClangASTMetadata *metadata = nullptr, + bool exports_symbols = false, + clang::SourceLocation location = clang::SourceLocation()); class TemplateParameterInfos { public: @@ -448,7 +458,7 @@ CompilerType CreateEnumerationType(const char *name, clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, - const Declaration &decl, + clang::SourceLocation loc, const CompilerType &integer_qual_type, bool is_scoped); @@ -939,11 +949,11 @@ // Modifying Enumeration types clang::EnumConstantDecl *AddEnumerationValueToEnumerationType( - const CompilerType &enum_type, const Declaration &decl, const char *name, - int64_t enum_value, uint32_t enum_value_bit_size); + const CompilerType &enum_type, clang::SourceLocation loc, + const char *name, int64_t enum_value, uint32_t enum_value_bit_size); clang::EnumConstantDecl *AddEnumerationValueToEnumerationType( - const CompilerType &enum_type, const Declaration &decl, const char *name, - const llvm::APSInt &value); + const CompilerType &enum_type, clang::SourceLocation loc, + const char *name, const llvm::APSInt &value); /// Returns the underlying integer type for an enum type. If the given type /// is invalid or not an enum-type, the function returns an invalid Index: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp =================================================================== --- lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp +++ lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp @@ -40,6 +40,7 @@ #include "clang/Sema/Sema.h" #include "llvm/Support/Signals.h" +#include "llvm/Support/SmallVectorMemoryBuffer.h" #include "llvm/Support/Threading.h" #include "Plugins/ExpressionParser/Clang/ClangASTImporter.h" @@ -716,6 +717,37 @@ Log *m_log; }; +/// Creates a dummy main file for the given SourceManager. +/// This file only serves as a container for include locations to other +/// FileIDs that are put into this type system (either by the ASTImporter +/// or when TypeSystemClang generates source locations for declarations). +static void CreateDummyMainFile(clang::SourceManager &sm, + clang::FileManager &fm) { + llvm::StringRef main_file_path = ""; + // The file contents are empty and should never be seen by the user. The new + // line is just there to not throw off any line counting logic that might + // expect files to end with a newline. + llvm::StringRef main_file_contents = "\n"; + const time_t mod_time = 0; + const off_t file_size = static_cast(main_file_contents.size()); + + // Create a virtual FileEntry for our dummy file. + const clang::FileEntry &fe = + *fm.getVirtualFile(main_file_path, file_size, mod_time); + + // Overwrite the file buffer with our empty file contents. + llvm::SmallVector buffer; + buffer.append(main_file_contents.begin(), main_file_contents.end()); + auto file_contents = std::make_unique( + std::move(buffer), main_file_path); + sm.overrideFileContents(&fe, std::move(file_contents)); + + // Create the actual file id for the FileEntry and set it as the main file. + clang::FileID fid = + sm.createFileID(&fe, SourceLocation(), clang::SrcMgr::C_User); + sm.setMainFileID(fid); +} + void TypeSystemClang::CreateASTContext() { assert(!m_ast_up); m_ast_owned = true; @@ -747,6 +779,8 @@ m_diagnostic_consumer_up = std::make_unique(); m_ast_up->getDiagnostics().setClient(m_diagnostic_consumer_up.get(), false); + CreateDummyMainFile(*m_source_manager_up, *m_file_manager_up); + // This can be NULL if we don't know anything about the architecture or if // the target for an architecture isn't enabled in the llvm/clang that we // built @@ -1280,7 +1314,8 @@ CompilerType TypeSystemClang::CreateRecordType( clang::DeclContext *decl_ctx, OptionalClangModuleID owning_module, AccessType access_type, llvm::StringRef name, int kind, - LanguageType language, ClangASTMetadata *metadata, bool exports_symbols) { + LanguageType language, ClangASTMetadata *metadata, bool exports_symbols, + SourceLocation location) { ASTContext &ast = getASTContext(); if (decl_ctx == nullptr) @@ -1304,6 +1339,8 @@ CXXRecordDecl *decl = CXXRecordDecl::CreateDeserialized(ast, 0); decl->setTagKind(static_cast(kind)); decl->setDeclContext(decl_ctx); + decl->setLocStart(location); + decl->setLocation(location); if (has_name) decl->setDeclName(&ast.Idents.get(name)); SetOwningModule(decl, owning_module); @@ -2163,14 +2200,82 @@ return CreateStructForIdentifier(type_name, type_fields, packed); } +/// A unique prefix that can never be found at the start of a valid C++ file. +/// Used to prefix content in LLDB-generated dummy files. +static const char *g_lldb_generated_source_prefix = + "getModificationTime() != g_lldb_generated_mod_time) + return false; + + // Check that the content of the file buffer has the prefix that all dummy + // files have. + llvm::StringRef buffer = sm.getBufferData(id); + return buffer.startswith(g_lldb_generated_source_prefix); +} + +SourceLocation TypeSystemClang::GetLocForDecl(const Declaration &decl) { + // If the Declaration is invalid there is nothing to do. + if (!decl.IsValid()) + return clang::SourceLocation(); + + clang::SourceManager &sm = getASTContext().getSourceManager(); + clang::FileManager &fm = sm.getFileManager(); + const std::string path = decl.GetFile().GetPath(); + + // Set modification time and content of the dummy file. This has to match + // the detection logic in IsDummyFileID. + // The contents of our dummy file. Will not be displayed to the user. + const std::string contents = g_lldb_generated_source_prefix + path + ">"; + // Get the virtual file entry for the given path. + const time_t mod_time = g_lldb_generated_mod_time; + + const off_t file_size = static_cast(contents.size()); + const clang::FileEntry &fe = *fm.getVirtualFile(path, file_size, mod_time); + + // Translate the file to a FileID. + clang::FileID fid = sm.translateFile(&fe); + if (fid.isInvalid()) { + // We see the file for the first time, so create a dummy file for it now. + llvm::SmallVector buffer; + buffer.append(contents.begin(), contents.end()); + auto file_contents = std::make_unique( + std::move(buffer), path); + sm.overrideFileContents(&fe, std::move(file_contents)); + + // Connect the new dummy file to the main file via some fake include + // location. This is necessary as all file's in the SourceManager need to be + // reachable via an include chain from the main file. + SourceLocation ToIncludeLocOrFakeLoc; + assert(sm.getMainFileID().isValid()); + ToIncludeLocOrFakeLoc = sm.getLocForStartOfFile(sm.getMainFileID()); + fid = sm.createFileID(&fe, ToIncludeLocOrFakeLoc, clang::SrcMgr::C_User); + + assert(IsDummyFileID(fid) && "Dummy file not detected by IsDummyFileID?"); + } + + // Return a SourceLocation at the start of the dummy file. We always return + // the first line/column as the dummy file contains just one line. + return sm.getLocForStartOfFile(fid); +} + #pragma mark Enumeration Types CompilerType TypeSystemClang::CreateEnumerationType( const char *name, clang::DeclContext *decl_ctx, - OptionalClangModuleID owning_module, const Declaration &decl, + OptionalClangModuleID owning_module, SourceLocation loc, const CompilerType &integer_clang_type, bool is_scoped) { - // TODO: Do something intelligent with the Declaration object passed in - // like maybe filling in the SourceLocation with it... ASTContext &ast = getASTContext(); // TODO: ask about these... @@ -2182,6 +2287,8 @@ enum_decl->setScoped(is_scoped); enum_decl->setScopedUsingClassTag(is_scoped); enum_decl->setFixed(false); + enum_decl->setLocStart(loc); + enum_decl->setLocation(loc); SetOwningModule(enum_decl, owning_module); if (enum_decl) { if (decl_ctx) @@ -8106,7 +8213,7 @@ } clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType( - const CompilerType &enum_type, const Declaration &decl, const char *name, + const CompilerType &enum_type, SourceLocation loc, const char *name, const llvm::APSInt &value) { if (!enum_type || ConstString(name).IsEmpty()) @@ -8140,6 +8247,7 @@ enumerator_decl->setDeclName(&getASTContext().Idents.get(name)); enumerator_decl->setType(clang::QualType(enutype, 0)); enumerator_decl->setInitVal(value); + enumerator_decl->setLocation(loc); SetMemberOwningModule(enumerator_decl, enutype->getDecl()); if (!enumerator_decl) @@ -8152,7 +8260,7 @@ } clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType( - const CompilerType &enum_type, const Declaration &decl, const char *name, + const CompilerType &enum_type, SourceLocation loc, const char *name, int64_t enum_value, uint32_t enum_value_bit_size) { CompilerType underlying_type = GetEnumerationIntegerType(enum_type); bool is_signed = false; @@ -8161,7 +8269,7 @@ llvm::APSInt value(enum_value_bit_size, is_signed); value = enum_value; - return AddEnumerationValueToEnumerationType(enum_type, decl, name, value); + return AddEnumerationValueToEnumerationType(enum_type, loc, name, value); } CompilerType TypeSystemClang::GetEnumerationIntegerType(CompilerType type) { Index: lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py =================================================================== --- lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py +++ lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py @@ -17,6 +17,7 @@ self.main_source = "main.cpp" self.main_source_spec = lldb.SBFileSpec(self.main_source) + self.main_source_diag_path = os.path.join("diagnostics", "main.cpp") def test_source_and_caret_printing(self): """Test that the source and caret positions LLDB prints are correct""" @@ -70,23 +71,13 @@ self.assertFalse(value.GetError().Success()) self.assertIn(":1:8: previous definition is here\nstruct SFoo{}; struct SFoo { int x; };\n ^\n", value.GetError().GetCString()) - # Declarations from the debug information currently have no debug information. It's not clear what - # we should do in this case, but we should at least not print anything that's wrong. - # In the future our declarations should have valid source locations. - value = frame.EvaluateExpression("struct FooBar { double x };", top_level_opts) - self.assertFalse(value.GetError().Success()) - self.assertEqual("error: :1:8: redefinition of 'FooBar'\nstruct FooBar { double x };\n ^\n", value.GetError().GetCString()) - - value = frame.EvaluateExpression("foo(1, 2)") - self.assertFalse(value.GetError().Success()) - self.assertEqual("error: :1:1: no matching function for call to 'foo'\nfoo(1, 2)\n^~~\nnote: candidate function not viable: requires single argument 'x', but 2 arguments were provided\n\n", value.GetError().GetCString()) - # Redefine something that we defined in a user-expression. We should use the previous expression file name # for the original decl. value = frame.EvaluateExpression("struct Redef { double x; };", top_level_opts) value = frame.EvaluateExpression("struct Redef { float y; };", top_level_opts) self.assertFalse(value.GetError().Success()) - self.assertIn("error: :1:8: redefinition of 'Redef'\nstruct Redef { float y; };\n ^\n:1:8: previous definition is here\nstruct Redef { double x; };\n ^", value.GetError().GetCString()) + self.assertIn("error: :1:8: redefinition of 'Redef'\nstruct Redef { float y; };\n ^\n", value.GetError().GetCString()) + self.assertIn(":1:8: previous definition is here\nstruct Redef { double x; };\n ^", value.GetError().GetCString()) @skipUnlessDarwin def test_source_locations_from_objc_modules(self): @@ -110,3 +101,116 @@ # the first argument are probably stable enough that this test can check for them. self.assertIn("void NSLog(NSString *format", value.GetError().GetCString()) + def assert_has_file_diagnostic(self, error_msg, file=None): + if file is None: + file = self.main_source_diag_path + # The SourceLocation for the file should be printed. + self.assertIn(file + ":", error_msg) + # Never show the dummy source code. This is the dummy content string + # used by TypeSystemClang. + self.assertNotIn("", error_msg) + + def assert_has_not_file_diagnostic(self, error_msg, file=None): + if file is None: + file = self.main_source_diag_path + # The SourceLocation should not be printed. + self.assertNotIn(file + ":", error_msg) + # Never show the dummy source code. This is the dummy content string + # used by TypeSystemClang. + self.assertNotIn("", error_msg) + + def test_source_locations_from_debug_information(self): + """Test that the source locations from debug information are correct""" + self.build() + + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, + '// Break here', self.main_source_spec) + frame = thread.GetFrameAtIndex(0) + top_level_opts = lldb.SBExpressionOptions(); + top_level_opts.SetTopLevel(True) + + # Test source locations of functions. + value = frame.EvaluateExpression("foo(1, 2)") + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assertIn(":1:1: no matching function for call to 'foo'\nfoo(1, 2)\n^~~", + value.GetError().GetCString()) + self.assert_has_file_diagnostic(value.GetError().GetCString()) + + # Test source locations of records. + value = frame.EvaluateExpression("struct FooBar { double x; };", top_level_opts) + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assertIn(":1:8: redefinition of 'FooBar'\nstruct FooBar { double x; };\n", + value.GetError().GetCString()) + self.assert_has_file_diagnostic(value.GetError().GetCString()) + + # Test source locations of enums. + value = frame.EvaluateExpression("enum class EnumInSource {};", top_level_opts) + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assert_has_file_diagnostic(value.GetError().GetCString()) + + # Test source locations in headers. + value = frame.EvaluateExpression("headerFunction(1)") + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assert_has_file_diagnostic(value.GetError().GetCString(), + "header.h") + + @skipIf(debug_info=no_match("dsym"), + bugnumber="Template function decl can only be found via dsym") + def test_source_locations_from_debug_information_templates(self): + """Test that the source locations from debug information are correct + for template functions""" + self.build() + + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, + '// Break here', self.main_source_spec) + frame = thread.GetFrameAtIndex(0) + + # Test source locations of template functions. + value = frame.EvaluateExpression("TemplateFunc(1)") + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assert_has_file_diagnostic(value.GetError().GetCString()) + + def test_disabled_source_locations(self): + """Test that disabling source locations with use-source-locations is + actually disabling the creation of valid source locations""" + self.build() + # Disable source locations. + self.runCmd("settings set plugin.symbol-file.dwarf.use-source-locations false") + + (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self, + '// Break here', self.main_source_spec) + frame = thread.GetFrameAtIndex(0) + top_level_opts = lldb.SBExpressionOptions(); + top_level_opts.SetTopLevel(True) + + # Functions shouldn't have source locations now. + value = frame.EvaluateExpression("foo(1, 2)") + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assertIn(":1:1: no matching function for call to 'foo'\nfoo(1, 2)\n^~~", + value.GetError().GetCString()) + self.assert_has_not_file_diagnostic(value.GetError().GetCString()) + + # Records shouldn't have source locations now. + value = frame.EvaluateExpression("struct FooBar { double x; };", top_level_opts) + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assertIn(":1:8: redefinition of 'FooBar'\nstruct FooBar { double x; };\n", + value.GetError().GetCString()) + self.assert_has_not_file_diagnostic(value.GetError().GetCString()) + + # Enums shouldn't have source locations now. + value = frame.EvaluateExpression("enum class EnumInSource {};", top_level_opts) + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assert_has_not_file_diagnostic(value.GetError().GetCString()) + + # Template functions shouldn't have source locations now. + value = frame.EvaluateExpression("TemplateFunc(1)") + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assert_has_not_file_diagnostic(value.GetError().GetCString()) + + # Test source locations in headers are disabled. + value = frame.EvaluateExpression("headerFunction(1)") + self.assertFalse(value.GetError().Success(), value.GetError().GetCString()) + self.assert_has_not_file_diagnostic(value.GetError().GetCString(), + "header.h") Index: lldb/test/API/commands/expression/diagnostics/header.h =================================================================== --- /dev/null +++ lldb/test/API/commands/expression/diagnostics/header.h @@ -0,0 +1 @@ +void headerFunction() {} Index: lldb/test/API/commands/expression/diagnostics/main.cpp =================================================================== --- lldb/test/API/commands/expression/diagnostics/main.cpp +++ lldb/test/API/commands/expression/diagnostics/main.cpp @@ -1,11 +1,20 @@ +#include "header.h" + void foo(int x) {} struct FooBar { int i; }; +enum class EnumInSource { A, B, C }; + +template void TemplateFunc() {} + int main() { FooBar f; foo(1); + EnumInSource e = EnumInSource::A; + TemplateFunc(); + headerFunction(); return 0; // Break here } Index: lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp =================================================================== --- lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp +++ lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp @@ -16,7 +16,7 @@ } C; } a; -// CHECK: A::(anonymous struct) +// CHECK: A::(anonymous struct at // CHECK: |-DefinitionData is_anonymous pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal -// CHECK: A::(anonymous struct) +// CHECK: A::(anonymous struct at // CHECK: |-DefinitionData pass_in_registers aggregate standard_layout trivially_copyable pod trivial literal Index: lldb/unittests/Symbol/TestTypeSystemClang.cpp =================================================================== --- lldb/unittests/Symbol/TestTypeSystemClang.cpp +++ lldb/unittests/Symbol/TestTypeSystemClang.cpp @@ -16,6 +16,9 @@ #include "clang/AST/DeclCXX.h" #include "clang/AST/DeclObjC.h" #include "clang/AST/ExprCXX.h" +#include "clang/Basic/FileManager.h" +#include "clang/Basic/SourceManager.h" +#include "llvm/Support/SmallVectorMemoryBuffer.h" #include "gtest/gtest.h" using namespace clang; @@ -44,6 +47,17 @@ return ClangUtil::GetQualType( m_ast->GetBuiltinTypeByName(ConstString(name))); } + + /// Returns the textual representation of the given source location. + std::string GetLocAsStr(clang::SourceLocation loc) { + return loc.printToString(m_ast->getASTContext().getSourceManager()); + } + + /// Returns the textual representation of the given Declaration's source + /// location. + std::string GetDeclLocAsStr(const lldb_private::Declaration &decl) { + return GetLocAsStr(m_ast->GetLocForDecl(decl)); + } }; TEST_F(TestTypeSystemClang, TestGetBasicTypeFromEnum) { @@ -260,8 +274,7 @@ CompilerType enum_type = ast.CreateEnumerationType( "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(), - Declaration(), basic_compiler_type, scoped); - + SourceLocation(), basic_compiler_type, scoped); CompilerType t = ast.GetEnumerationIntegerType(enum_type); // Check that the type we put in at the start is found again. EXPECT_EQ(basic_compiler_type.GetTypeName(), t.GetTypeName()); @@ -274,7 +287,7 @@ CompilerType basic_compiler_type = ast.GetBasicType(BasicType::eBasicTypeInt); CompilerType enum_type = ast.CreateEnumerationType( "my_enum", ast.GetTranslationUnitDecl(), OptionalClangModuleID(100), - Declaration(), basic_compiler_type, false); + clang::SourceLocation(), basic_compiler_type, false); auto *ed = TypeSystemClang::GetAsEnumDecl(enum_type); EXPECT_FALSE(!ed); EXPECT_EQ(ed->getOwningModuleID(), 100u); @@ -722,3 +735,70 @@ EXPECT_FALSE(method->isDirectMethod()); EXPECT_EQ(method->getDeclName().getObjCSelector().getAsString(), "foo"); } + +/// Small utility function to create a Declaration with a given line without +/// having to write down the other parameters that don't matter for most tests. +static lldb_private::Declaration DeclAtLineAndCol(uint32_t line, uint32_t col) { + return Declaration(FileSpec("main.cpp"), line, col); +} + +static const char *invalid_loc = ""; + +TEST_F(TestTypeSystemClang, TestGetLocForDecl_InvalidDeclaration) { + // Invalid Declaration should produce an invalid location. + lldb_private::Declaration decl; + EXPECT_EQ(invalid_loc, GetDeclLocAsStr(decl)); +} + +TEST_F(TestTypeSystemClang, TestGetLocForDecl) { + // Test that GetLocForDecl returns a valid source location pointing + // to the start of a dummy file with the same path. + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(1, 0))); + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(2, 0))); + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(3, 0))); + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(1000, 0))); + + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(1, 1))); + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(1, 2))); + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(1, 3))); + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(1, 1000))); + + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(2, 1))); + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(2, 2))); + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(2, 3))); + EXPECT_EQ("main.cpp:1:1", GetDeclLocAsStr(DeclAtLineAndCol(2, 1000))); +} + +TEST_F(TestTypeSystemClang, TestGetLocForDecl_BufferContent) { + // Test that GetLocForDecl returns a valid source location pointing + // to the start of a dummy file with the same path. + SourceLocation loc = m_ast->GetLocForDecl(DeclAtLineAndCol(1, 0)); + clang::SourceManager &sm = m_ast->getASTContext().getSourceManager(); + EXPECT_EQ("", + sm.getBufferData(sm.getFileID(loc))); +} + +TEST_F(TestTypeSystemClang, TestGetLocForDecl_MultipleLocs) { + // Test having multiple source files passed to GetLocForDecl. + Declaration main_decl(FileSpec("main.cpp"), 2, 1); + SourceLocation main_loc = m_ast->GetLocForDecl(main_decl); + EXPECT_EQ("main.cpp:1:1", GetLocAsStr(main_loc)); + + Declaration header_decl(FileSpec("some_header.h"), 1, 1); + SourceLocation header_loc = m_ast->GetLocForDecl(header_decl); + EXPECT_EQ("some_header.h:1:1", GetLocAsStr(header_loc)); + + Declaration other_header_decl(FileSpec("other_header"), 1, 1); + SourceLocation other_header_loc = m_ast->GetLocForDecl(other_header_decl); + EXPECT_EQ("other_header:1:1", GetLocAsStr(other_header_loc)); + + // This tests that the file tree we create in the Clang SourceManager is + // allowing Clang to establish a deterministic ordering between all the source + // locations. If this wasn't the case, then isBeforeInTranslationUnit (which + // is for example used by Clang when ordering diagnostics) would assert. + clang::SourceManager &SM = m_ast->getASTContext().getSourceManager(); + SM.isBeforeInTranslationUnit(header_loc, main_loc); + SM.isBeforeInTranslationUnit(main_loc, header_loc); + SM.isBeforeInTranslationUnit(header_loc, other_header_loc); + SM.isBeforeInTranslationUnit(main_loc, other_header_loc); +}