[lldb] Add valid SourceLocations to Clang declarations generated by the DWARF parser

[teemperor]

Summary

Clang supports having SourceLocations added to most declarations which are used for error diagnostics
to let the user know from which file/line a certain declaration came from.

Currently we assign all Clang declarations that are generated by the DWARF parser an invalid
SourceLocation, which leads to confusing error messages that tell the user things such as "previously declared here:"
but not any file path or line number that tells the user where "here" is.

This patch partly fixes the issue by assigning all declarations generated by the DWARF parser a valid SourceLocation.
The SourceLocations that are generated all point to dummy files that are just located in memory and just contain
the string <content of /path/to/file.cpp>. This is just done to keep this patch simple, but the idea is that in the
future we open the actual source files and let the SourceLocations point there to the right line/column. For now
we just display the file path which is the only thing we get correct for now.

The new diagnostics look like this:

(lldb) expr --top-level -- struct Foo {};
error: <user expression 0>:1:8: redefinition of 'Foo'
struct Foo {};
       ^
/path/to/test/loc.cpp: previous definition is here

The old diagnostics are the same but are missing the part before and after the "previous definition is here" error.

This feature can be turned off by setting plugin.symbol-file.dwarf.use-source-locations to false.

Partly fixes rdar://8670536

[teemperor]

• Added test for source locations from a header file.

[labath]

This seems relatively ok-ish, though I don't really know much about this stuff. It may be good if we could get some of the clang-folks to review this, to make sure we don't do something too unsupported, which would be hard to back out from...

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

2206–2207

Random thought: would this be cleaner if we always created a main file somewhere, which would not correspond to any of the real files we care about, but would just serve as a place to hook the other files onto?

8109

clang-format ?

lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

103

I suspect this path will be a problem on windows

lldb/test/API/commands/expression/diagnostics/main.cpp

2

Is this deliberately on the second line?

[shafik]

So if I understand correctly, everything will now have 1:1 for their location?

[teemperor]

In D78095#1989879, @shafik wrote:

So if I understand correctly, everything will now have 1:1 for their location?

Kinda, it's /path/to/whatever/file/they/came/from.bar:1:1.

[teemperor]

After some discussion with Shafik we ended up deciding that we should also hide the :1:1 in the diagnostic to not give the impression that this is a real line/col pair.

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

2206–2207

That's actually a good idea, I'll change that.

[teemperor]

• Now creating one dummy main file when the ASTContext is created.
• Now hiding line/columns and generated source code when rendering diagnostics.

[aprantl]

Currently I don't think the DW_AT_decl_line / decl_file in DWARF type declarations are used by anything. In the Swift compiler actually started removing some of them since they caused needless churn in incremental recompilation, when someone inserted a blank line in a source file, for example. I was thinking we might want to do something similar for Clang, too. This patch would be an argument for not doing this. Since C-based languages separate interfaces into header files, the recompilation whitespace problem isn't quite as dramatic as in Swift, either. I suppose we can do this :-)

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

1367

nit: I usually spell the default return value as return {} but there's no LLVM coding style rule for this.

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.h

lldb::TypeSP ParseEnum(const lldb_private::SymbolContext &sc,
                          const DWARFDIE &die, ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP ParseSubroutine(const DWARFDIE &die,
                                ParsedDWARFTypeAttributes &attrs);
   // FIXME: attrs should be passed as a const reference.
   lldb::TypeSP ParseArrayType(const DWARFDIE &die,
                               ParsedDWARFTypeAttributes &attrs);
   lldb::TypeSP ParsePointerToMemberType(const DWARFDIE &die,
                                         const ParsedDWARFTypeAttributes &attrs);
+
+  /// 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.
 /// Some attributes are relevant for all kinds of types (declaration), while
 /// others are only meaningful to a specific type (is_virtual)
 struct ParsedDWARFTypeAttributes {
   explicit ParsedDWARFTypeAttributes(const DWARFDIE &die);
 

lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp

if (!enumerator_clang_type) {
             "", DW_ATE_signed, *attrs.byte_size * 8);
       } else {
         enumerator_clang_type = m_ast.GetBasicType(eBasicTypeInt);
       }
     }
 
     clang_type = m_ast.CreateEnumerationType(
         attrs.name.GetCString(), GetClangDeclContextContainingDIE(die, nullptr),
-        GetOwningClangModule(die), attrs.decl, enumerator_clang_type,
+        GetOwningClangModule(die), GetLocForDecl(die, attrs.decl), enumerator_clang_type,
         attrs.is_scoped_enum);
   } else {
     enumerator_clang_type = m_ast.GetEnumerationIntegerType(clang_type);
   }
 
   LinkDeclContextToDIE(TypeSystemClang::GetDeclContextForType(clang_type), die);
 
   type_sp = std::make_shared<Type>(
if (!type_handled) {
         }
 
         // We just have a function that isn't part of a class
         function_decl = m_ast.CreateFunctionDeclaration(
             ignore_containing_context ? m_ast.GetTranslationUnitDecl()
                                       : 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;
           ParseTemplateParameterInfos(die, template_param_infos);
           template_function_decl = m_ast.CreateFunctionDeclaration(
               ignore_containing_context ? m_ast.GetTranslationUnitDecl()
                                         : 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);
         }
 
         lldbassert(function_decl);
 
         if (function_decl) {
           LinkDeclContextToDIE(function_decl, die);
if (llvm::Optional<uint64_t> clang_type_size =
     return std::make_shared<Type>(die.GetID(), dwarf, attrs.name,
                                   *clang_type_size, nullptr, LLDB_INVALID_UID,
                                   Type::eEncodingIsUID, nullptr, clang_type,
                                   Type::ResolveState::Forward);
   }
   return nullptr;
 }
 
+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)
     return type_sp;
 
   SymbolFileDWARF *dwarf = die.GetDWARF();
   TypeList &type_list = dwarf->GetTypeList();
   DWARFDIE sc_parent_die = SymbolFileDWARF::GetParentSymbolContextDIE(die);
if (attrs.name.GetStringRef().contains('<')) {
           }
           return TypeSP();
         }
 
         clang::ClassTemplateSpecializationDecl *class_specialization_decl =
             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;
 
         m_ast.SetMetadata(class_template_decl, metadata);
         m_ast.SetMetadata(class_specialization_decl, metadata);
       }
     }
 
     if (!clang_type_was_created) {
       clang_type_was_created = true;
       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));
     }
   }
 
   // Store a forward declaration to this class type in case any
   // parameters in any class methods need it for the clang types for
   // function prototypes.
   LinkDeclContextToDIE(m_ast.GetDeclContextForType(clang_type), die);
   type_sp = std::make_shared<Type>(
if (tag == DW_TAG_enumerator) {
             case DW_AT_sibling:
               break;
             }
           }
         }
 
         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;
         }
       }
     }
   }
   return enumerators_added;
 }
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h

public:
 
   static bool
   DIEInDeclContext(const lldb_private::CompilerDeclContext &parent_decl_ctx,
                    const DWARFDIE &die);
 
   std::vector<std::unique_ptr<lldb_private::CallEdge>>
   ParseCallEdgesInFunction(UserID func_id) override;
 
+  bool UseSourceLocations() const;
+
   void Dump(lldb_private::Stream &s) override;
 
   void DumpClangAST(lldb_private::Stream &s) override;
 
   lldb_private::DWARFContext &GetDWARFContext() { return m_context; }
 
   const std::shared_ptr<SymbolFileDWARFDwo> &GetDwpSymbolFile();
 

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp

PluginProperties() {
     m_collection_sp = std::make_shared<OptionValueProperties>(GetSettingName());
     m_collection_sp->Initialize(g_symbolfiledwarf_properties);
   }
 
   bool IgnoreFileIndexes() const {
     return m_collection_sp->GetPropertyAtIndexAsBoolean(
         nullptr, ePropertyIgnoreIndexes, false);
   }
+
+  bool UseSourceLocations() const {
+    return m_collection_sp->GetPropertyAtIndexAsBoolean(
+        nullptr, ePropertyUseSourceLocations, false);
+  }
 };
 
 typedef std::shared_ptr<PluginProperties> SymbolFileDWARFPropertiesSP;
 
 static const SymbolFileDWARFPropertiesSP &GetGlobalPluginProperties() {
   static const auto g_settings_sp(std::make_shared<PluginProperties>());
   return g_settings_sp;
 }

 std::vector<std::unique_ptr<lldb_private::CallEdge>>
 SymbolFileDWARF::ParseCallEdgesInFunction(UserID func_id) {
   DWARFDIE func_die = GetDIE(func_id.GetID());
   if (func_die.IsValid())
     return CollectCallEdges(GetObjectFile()->GetModule(), func_die);
   return {};
 }
 
+bool SymbolFileDWARF::UseSourceLocations() const {
+  return GetGlobalPluginProperties()->UseSourceLocations();
+}
+
 // PluginInterface protocol
 ConstString SymbolFileDWARF::GetPluginName() { return GetPluginNameStatic(); }
 
 uint32_t SymbolFileDWARF::GetPluginVersion() { return 1; }
 
 void SymbolFileDWARF::Dump(lldb_private::Stream &s) {
   SymbolFile::Dump(s);
   m_index->Dump(s);

lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFProperties.td

 include "../../../../include/lldb/Core/PropertiesBase.td"
 
 let Definition = "symbolfiledwarf" in {
   def IgnoreIndexes: Property<"ignore-file-indexes", "Boolean">,
     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.">;
 }

lldb/source/Plugins/SymbolFile/NativePDB/PdbAstBuilder.cpp

clang::QualType PdbAstBuilder::CreateEnumType(PdbTypeSymId id,
                                               const EnumRecord &er) {
   clang::DeclContext *decl_context = nullptr;
   std::string uname;
   std::tie(decl_context, uname) = CreateDeclInfoForType(er, id.index);
   clang::QualType underlying_type = GetOrCreateType(er.UnderlyingType);
 
   Declaration declaration;
   CompilerType enum_ct = m_clang.CreateEnumerationType(
-      uname.c_str(), decl_context, OptionalClangModuleID(), declaration,
+      uname.c_str(), decl_context, OptionalClangModuleID(), clang::SourceLocation(),
       ToCompilerType(underlying_type), er.isScoped());
 
   TypeSystemClang::StartTagDeclarationDefinition(enum_ct);
   TypeSystemClang::SetHasExternalStorage(enum_ct.GetOpaqueQualType(), true);
 
   return clang::QualType::getFromOpaquePtr(enum_ct.GetOpaqueQualType());
 }
 

lldb/source/Plugins/SymbolFile/NativePDB/UdtRecordCompleter.cpp

 }
 
 Error UdtRecordCompleter::visitKnownMember(CVMemberRecord &cvr,
                                            EnumeratorRecord &enumerator) {
   Declaration decl;
   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();
 }
 
 void UdtRecordCompleter::complete() {
   // Ensure the correct order for virtual bases.
   std::stable_sort(m_bases.begin(), m_bases.end(),
                    [](const IndexedBase &lhs, const IndexedBase &rhs) {
                      return lhs.first < rhs.first;

lldb/source/Plugins/SymbolFile/PDB/PDBASTParser.cpp

if (!ast_enum.IsValid()) {
       else
         builtin_type = m_ast.GetBasicType(eBasicTypeInt);
 
       // FIXME: PDB does not have information about scoped enumeration (Enum
       // Class). Set it false for now.
       bool isScoped = false;
 
       ast_enum = m_ast.CreateEnumerationType(name.c_str(), decl_context,
-                                             OptionalClangModuleID(), decl,
+                                             OptionalClangModuleID(),
+                                             clang::SourceLocation(),
                                              builtin_type, isScoped);
 
       auto enum_decl = TypeSystemClang::GetAsEnumDecl(ast_enum);
       assert(enum_decl);
       m_uid_to_decl[type.getSymIndexId()] = enum_decl;
 
       auto enum_values = enum_type->findAllChildren<PDBSymbolData>();
       if (enum_values) {
case PDB_VariantType::UInt64:
     break;
   default:
     return false;
   }
   CompilerType underlying_type = m_ast.GetEnumerationIntegerType(enum_type);
   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;
 
   m_uid_to_decl[enum_value.getSymIndexId()] = enum_constant_decl;
 
   return true;
 }
 

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.h

CompilerType CreateStructForIdentifier(
       bool packed = false);
 
   CompilerType GetOrCreateStructForIdentifier(
       ConstString type_name,
       const std::initializer_list<std::pair<const char *, CompilerType>>
           &type_fields,
       bool packed = false);
 
+  /// 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);
 
   // Structure, Unions, Classes
 
   static clang::AccessSpecifier
   ConvertAccessTypeToAccessSpecifier(lldb::AccessType access);
 
OptionalClangModuleID GetOrCreateClangModule(llvm::StringRef name,
                                                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);
+                                bool exports_symbols = false,
+                                clang::SourceLocation location = clang::SourceLocation());
 
   class TemplateParameterInfos {
   public:
     bool IsValid() const {
       if (args.empty())
         return false;
       return args.size() == names.size() &&
         ((bool)pack_name == (bool)packed_args) &&
public:
 
   CompilerType CreateArrayType(const CompilerType &element_type,
                                size_t element_count, bool is_vector);
 
   // Enumeration Types
   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);
 
   // Integer type functions
 
   CompilerType GetIntTypeFromBitSize(size_t bit_size, bool is_signed);
 
   CompilerType GetPointerSizedIntType(bool is_signed);
#define LLDB_INVALID_DECL_LEVEL UINT32_MAX
 
   // Tag Declarations
   static bool StartTagDeclarationDefinition(const CompilerType &type);
 
   static bool CompleteTagDeclarationDefinition(const CompilerType &type);
 
   // Modifying Enumeration types
   clang::EnumConstantDecl *AddEnumerationValueToEnumerationType(
-      const CompilerType &enum_type, const Declaration &decl, const char *name,
+      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 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
   /// CompilerType.
   CompilerType GetEnumerationIntegerType(CompilerType type);
 
   // Pointers & References

lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp

 #include "clang/Basic/TargetOptions.h"
 #include "clang/Frontend/FrontendOptions.h"
 #include "clang/Lex/HeaderSearch.h"
 #include "clang/Lex/HeaderSearchOptions.h"
 #include "clang/Lex/ModuleMap.h"
 #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"
 #include "Plugins/ExpressionParser/Clang/ClangASTMetadata.h"
 #include "Plugins/ExpressionParser/Clang/ClangExternalASTSourceCallbacks.h"
 #include "Plugins/ExpressionParser/Clang/ClangFunctionCaller.h"
 #include "Plugins/ExpressionParser/Clang/ClangPersistentVariables.h"
 #include "Plugins/ExpressionParser/Clang/ClangUserExpression.h"
if (!created)
     return ast_source->GetIDForModule(module);
 
   return ast_source->RegisterModule(module);
 }
 
 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)
     decl_ctx = ast.getTranslationUnitDecl();
 
   if (language == eLanguageTypeObjC ||
       language == eLanguageTypeObjC_plus_plus) {
     bool isForwardDecl = true;
     bool isInternal = false;
     return CreateObjCClass(name, decl_ctx, owning_module, isForwardDecl,
                            isInternal, metadata);
   }
 
   // NOTE: Eventually CXXRecordDecl will be merged back into RecordDecl and
   // we will need to update this code. I was told to currently always use the
   // CXXRecordDecl class since we often don't know from debug information if
   // something is struct or a class, so we default to always use the more
   // complete definition just in case.
 
   bool has_name = !name.empty();
   CXXRecordDecl *decl = CXXRecordDecl::CreateDeserialized(ast, 0);
   decl->setTagKind(static_cast<TagDecl::TagKind>(kind));
   decl->setDeclContext(decl_ctx);
+  decl->setLocStart(location);
+  decl->setLocation(location);
   if (has_name)
     decl->setDeclName(&ast.Idents.get(name));
   SetOwningModule(decl, owning_module);
 
   if (!has_name) {
     // In C++ a lambda is also represented as an unnamed class. This is
     // different from an *anonymous class* that the user wrote:
     //
CompilerType TypeSystemClang::GetOrCreateStructForIdentifier(
     bool packed) {
   CompilerType type;
   if ((type = GetTypeForIdentifier<clang::CXXRecordDecl>(type_name)).IsValid())
     return type;
 
   return CreateStructForIdentifier(type_name, type_fields, packed);
 }
 
+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();
+  // The contents of our dummy file. Contains a string that will be displayed
+  // to the user in error diagnostics.
+  const std::string contents = "<content of " + path + ">";
+
+  // Get the virtual file entry for the given path.
+  const time_t mod_time = 0;
+  const off_t file_size = static_cast<off_t>(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<char, 64> buffer;
+    buffer.append(contents.begin(), contents.end());
+    auto file_contents = std::make_unique<llvm::SmallVectorMemoryBuffer>(
+        std::move(buffer), path);
+    sm.overrideFileContents(&fe, std::move(file_contents));
+
+    // If this isn't the first dummy file we created, connect the new dummy
+    // file to the first 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;
+    if (sm.getMainFileID().isValid())
+      ToIncludeLocOrFakeLoc = sm.getLocForStartOfFile(sm.getMainFileID());
+    fid = sm.createFileID(&fe, ToIncludeLocOrFakeLoc, clang::SrcMgr::C_User);
+    // The first dummy file will be marked as the main source file. It doesn't
+    // really matter what is the main file in LLDB's use case as this is
+    // primarily used for rewriting source code via Clang.
+    if (sm.getMainFileID().isInvalid())
+      sm.setMainFileID(fid);
+  }
+
+  // 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...
   //    const bool IsFixed = false;
   EnumDecl *enum_decl = EnumDecl::CreateDeserialized(ast, 0);
   enum_decl->setDeclContext(decl_ctx);
   if (name && name[0])
     enum_decl->setDeclName(&ast.Idents.get(name));
   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)
       decl_ctx->addDecl(enum_decl);
 
     // TODO: check if we should be setting the promotion type too?
     enum_decl->setIntegerType(ClangUtil::GetQualType(integer_clang_type));
 
if (!integer_type.isNull()) {
     enum_decl->completeDefinition(enum_decl->getIntegerType(),
                                   promotion_qual_type, NumPositiveBits,
                                   NumNegativeBits);
   }
   return true;
 }
 
 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())
     return nullptr;
 
   lldbassert(enum_type.GetTypeSystem() == static_cast<TypeSystem *>(this));
 
   lldb::opaque_compiler_type_t enum_opaque_compiler_type =
clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType(
 
   clang::EnumConstantDecl *enumerator_decl =
       clang::EnumConstantDecl::CreateDeserialized(getASTContext(), 0);
   enumerator_decl->setDeclContext(enutype->getDecl());
   if (name && name[0])
     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)
     return nullptr;
 
   enutype->getDecl()->addDecl(enumerator_decl);
 
   VerifyDecl(enumerator_decl);
   return enumerator_decl;
 }
 
-clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType(
-    const CompilerType &enum_type, const Declaration &decl, const char *name,
+clang::EnumConstantDecl *TypeSystemClang::AddEnumerationValueToEnumerationType(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;
   underlying_type.IsIntegerType(is_signed);
 
   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) {
   clang::QualType qt(ClangUtil::GetQualType(type));
   const clang::Type *clang_type = qt.getTypePtrOrNull();
   const auto *enum_type = llvm::dyn_cast_or_null<clang::EnumType>(clang_type);
   if (!enum_type)
     return CompilerType();

lldb/test/API/commands/expression/diagnostics/TestExprDiagnostics.py

def test_source_and_caret_printing(self):
         self.assertIn("\nvoid foo;\n     ^", value.GetError().GetCString())
         self.assertIn("<user expression 4>:2:6", value.GetError().GetCString())
 
         # Test that we render Clang's 'notes' correctly.
         value = frame.EvaluateExpression("struct SFoo{}; struct SFoo { int x; };", top_level_opts)
         self.assertFalse(value.GetError().Success())
         self.assertIn("<user expression 5>: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: <user expression 6>: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: <user expression 7>: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: <user expression 9>:1:8: redefinition of 'Redef'\nstruct Redef { float y; };\n       ^\n<user expression 8>:1:8: previous definition is here\nstruct Redef { double x; };\n       ^", value.GetError().GetCString())
+        self.assertIn("error: <user expression 7>:1:8: redefinition of 'Redef'\nstruct Redef { float y; };\n       ^\n", value.GetError().GetCString())
+        self.assertIn("<user expression 6>:1:8: previous definition is here\nstruct Redef { double x; };\n       ^", value.GetError().GetCString())
 
     @skipUnlessDarwin
     def test_source_locations_from_objc_modules(self):
         self.build()
 
         (target, process, thread, bkpt) = lldbutil.run_to_source_breakpoint(self,
                                           '// Break here', self.main_source_spec)
         frame = thread.GetFrameAtIndex(0)
 
         # Import foundation so that the Obj-C module is loaded (which contains source locations
         # that can be used by LLDB).
         self.runCmd("expr @import Foundation")
         value = frame.EvaluateExpression("NSLog(1);")
         self.assertFalse(value.GetError().Success())
         # LLDB should print the source line that defines NSLog. To not rely on any
         # header paths/line numbers or the actual formatting of the Foundation headers, only look
         # for a few tokens in the output.
         # File path should come from Foundation framework.
         self.assertIn("/Foundation.framework/", value.GetError().GetCString())
         # The NSLog definition source line should be printed. Return value and
         # 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="diagnostics/main.cpp"):
+        self.assertIn(file + ":1:1:", error_msg)
+        self.assertIn("\n<content of", error_msg)
+        self.assertIn(file + ">\n^\n", error_msg)
+
+    def assert_has_not_file_diagnostic(self, error_msg, file="diagnostics/main.cpp"):
+        self.assertNotIn(file + ":1:1:", error_msg)
+        self.assertNotIn("\n<content of", error_msg)
+        self.assertNotIn(file + ">\n^\n", 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<int>(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<int>(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")

lldb/test/API/commands/expression/diagnostics/header.h

+void headerFunction() {}

lldb/test/API/commands/expression/diagnostics/main.cpp

+
+#include "header.h"
 void foo(int x) {}
 
 struct FooBar {
   int i;
 };
 
+enum class EnumInSource { A, B, C };
+
+template <typename T> void TemplateFunc() {}
+
 int main() {
   FooBar f;
   foo(1);
+  EnumInSource e = EnumInSource::A;
+  TemplateFunc<int>();
+  headerFunction();
   return 0; // Break here
 }

lldb/test/Shell/SymbolFile/DWARF/clang-ast-from-dwarf-unamed-and-anon-structs.cpp

struct A {
   struct {
     int x;
   };
   struct {
     int y;
   } 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

lldb/unittests/Symbol/TestTypeSystemClang.cpp

 #include "TestingSupport/SubsystemRAII.h"
 #include "TestingSupport/Symbol/ClangTestUtils.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/Declaration.h"
 #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;
 using namespace lldb;
 using namespace lldb_private;
 
 class TestTypeSystemClang : public testing::Test {
 public:
protected:
   QualType GetBasicQualType(BasicType type) const {
     return ClangUtil::GetQualType(m_ast->GetBasicTypeFromAST(type));
   }
 
   QualType GetBasicQualType(const char *name) const {
     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) {
   clang::ASTContext &context = m_ast->getASTContext();
 
   EXPECT_TRUE(
       context.hasSameType(GetBasicQualType(eBasicTypeBool), context.BoolTy));
   EXPECT_TRUE(
for (lldb::BasicType basic_type : types_to_test) {
       SCOPED_TRACE(std::to_string(basic_type));
 
       TypeSystemClang ast("enum_ast", HostInfo::GetTargetTriple());
       CompilerType basic_compiler_type = ast.GetBasicType(basic_type);
       EXPECT_TRUE(basic_compiler_type.IsValid());
 
       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());
     }
   }
 }
 
 TEST_F(TestTypeSystemClang, TestOwningModule) {
   TypeSystemClang ast("module_ast", HostInfo::GetTargetTriple());
   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);
 
   CompilerType record_type = ast.CreateRecordType(
       nullptr, OptionalClangModuleID(200), lldb::eAccessPublic, "FooRecord",
       clang::TTK_Struct, lldb::eLanguageTypeC_plus_plus, nullptr);
   auto *rd = TypeSystemClang::GetAsRecordDecl(record_type);
m_ast->AddMethodToCXXRecordType(
         is_explicit, is_attr_used, is_artificial);
   }
 
   // Complete the definition and check the created record.
   m_ast->CompleteTagDeclarationDefinition(t);
   auto *record = llvm::cast<CXXRecordDecl>(ClangUtil::GetAsTagDecl(t));
   EXPECT_TRUE(record->hasUserDeclaredCopyConstructor());
 }
+
+/// 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 = "<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("<content of main.cpp>", 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);
+}