Update SymbolList<>::create_symbol function, tweak docs/comments

docs/SymbolDatabase.md

@@ -37,9 +37,9 @@ together using shared pointers:
 The symbol list classes maintain maps that can be used to lookup a symbol by its
 address or name where that makes sense.
 
-In addition, you can lookup a symbol from an address that is contained by said
-symbol. For example, it is possible to find the function that an instruction
-belongs to by looking up its address.
+In addition, you can lookup a symbol from an address that it overlaps. For
+example, it is possible to find the function that an instruction belongs to by
+looking up its address.
 
 ## Modules
 
@@ -116,4 +116,5 @@ sum += functions.size();
 ...
 ```
 
-This way, if we wanted to add another type of symbol, we wouldn't have to modify the symbol count code above.
+This way, if we wanted to add another type of symbol, we wouldn't have to modify
+the symbol count code above.

src/ccc/elf.cpp

@@ -152,7 +152,7 @@ Result<void> import_elf_section_headers(
 	SymbolDatabase& database, const ElfFile& elf, SymbolSourceHandle source, const Module* module_symbol)
 {
 	for(const ElfSection& section : elf.sections) {
-		Result<Section*> symbol = database.sections.create_symbol(section.name, source, module_symbol, section.address);
+		Result<Section*> symbol = database.sections.create_symbol(section.name, section.address, source, module_symbol);
 		CCC_RETURN_IF_ERROR(symbol);
 
 		(*symbol)->set_size(section.size);

src/ccc/mdebug_analysis.cpp

@@ -200,7 +200,7 @@ Result<void> LocalSymbolTableAnalyser::local_variable(
 
 	Address address = (desc == StabsSymbolDescriptor::STATIC_LOCAL_VARIABLE) ? value : Address();
 	Result<LocalVariable*> local_variable = m_database.local_variables.create_symbol(
-		name, m_context.symbol_source, m_context.module_symbol, address);
+		name, address, m_context.symbol_source, m_context.module_symbol);
 	CCC_RETURN_IF_ERROR(local_variable);
 
 	m_current_local_variables.expand_to_include((*local_variable)->handle());

src/ccc/mdebug_importer.cpp

@@ -144,7 +144,7 @@ Result<void> import_file(SymbolDatabase& database, const mdebug::File& input, co
 	}
 
 	Result<SourceFile*> source_file = database.source_files.create_symbol(
-		input.full_path, context.symbol_source, context.module_symbol, text_address);
+		input.full_path, text_address, context.symbol_source, context.module_symbol);
 	CCC_RETURN_IF_ERROR(source_file);
 
 	(*source_file)->working_dir = input.working_dir;

src/ccc/symbol_database.cpp

@@ -199,6 +199,55 @@ s32 SymbolList<SymbolType>::size() const
 	return (s32) m_symbols.size();
 }
 
+template <typename SymbolType>
+Result<SymbolType*> SymbolList<SymbolType>::create_symbol(
+	std::string name, Address address, SymbolSourceHandle source, const Module* module_symbol)
+{
+	CCC_CHECK(m_next_handle != UINT32_MAX, "Ran out of handles to use for %s symbols.", SymbolType::NAME);
+
+	u32 handle = m_next_handle++;
+
+	SymbolType& symbol = m_symbols.emplace_back();
+
+	symbol.m_handle = handle;
+	symbol.m_name = std::move(name);
+
+	if constexpr(std::is_same_v<SymbolType, SymbolSource>) {
+		// It doesn't make sense for the calling code to provide a symbol source
+		// handle as an argument if we're creating a symbol source symbol, so we
+		// set the source of the new symbol to its own handle.
+		symbol.m_source = handle;
+	} else {
+		CCC_ASSERT(source.valid());
+		symbol.m_source = source;
+	}
+
+	if constexpr(std::is_same_v<SymbolType, Module>) {
+		// It doesn't make sense for the calling code to provide a module as an
+		// argument if we're creating a module symbol, so we set the module of
+		// the new symbol to its own handle.
+		symbol.m_address = address;
+		symbol.m_module = handle;
+	} else if(module_symbol) {
+		symbol.m_address = address.add_base_address(module_symbol->address());
+		symbol.m_module = module_symbol->handle();
+	} else {
+		symbol.m_address = address;
+	}
+
+	link_address_map(symbol);
+	link_name_map(symbol);
+
+	return &symbol;
+}
+
+template <typename SymbolType>
+Result<SymbolType*> SymbolList<SymbolType>::create_symbol(
+	std::string name, SymbolSourceHandle source, const Module* module_symbol)
+{
+	return create_symbol(name, Address(), source, module_symbol);
+}
+
 template <typename SymbolType>
 Result<SymbolType*> SymbolList<SymbolType>::create_symbol(
 	std::string name, SymbolSourceHandle source, const Module* module_symbol, Address address, u32 importer_flags, DemanglerFunctions demangler)
@@ -234,7 +283,7 @@ Result<SymbolType*> SymbolList<SymbolType>::create_symbol(
 		}
 	}
 
-	Result<SymbolType*> symbol = create_symbol(non_mangled_name, source, module_symbol, address);
+	Result<SymbolType*> symbol = create_symbol(non_mangled_name, address, source, module_symbol);
 	CCC_RETURN_IF_ERROR(symbol);
 
 	if constexpr(SymbolType::FLAGS & NAME_NEEDS_DEMANGLING) {
@@ -246,48 +295,6 @@ Result<SymbolType*> SymbolList<SymbolType>::create_symbol(
 	return symbol;
 }
 
-template <typename SymbolType>
-Result<SymbolType*> SymbolList<SymbolType>::create_symbol(
-	std::string name, SymbolSourceHandle source, const Module* module_symbol, Address address)
-{
-	CCC_CHECK(m_next_handle != UINT32_MAX, "Ran out of handles to use for %s symbols.", SymbolType::NAME);
-
-	u32 handle = m_next_handle++;
-
-	SymbolType& symbol = m_symbols.emplace_back();
-
-	symbol.m_handle = handle;
-	symbol.m_name = std::move(name);
-
-	if constexpr(std::is_same_v<SymbolType, SymbolSource>) {
-		// It doesn't make sense for the calling code to provide a symbol source
-		// handle as an argument if we're creating a symbol source symbol, so we
-		// set the source of the new symbol to its own handle.
-		symbol.m_source = handle;
-	} else {
-		CCC_ASSERT(source.valid());
-		symbol.m_source = source;
-	}
-
-	if constexpr(std::is_same_v<SymbolType, Module>) {
-		// It doesn't make sense for the calling code to provide a module as an
-		// argument if we're creating a module symbol, so we set the module of
-		// the new symbol to its own handle.
-		symbol.m_address = address;
-		symbol.m_module = handle;
-	} else if(module_symbol) {
-		symbol.m_address = address.add_base_address(module_symbol->address());
-		symbol.m_module = module_symbol->handle();
-	} else {
-		symbol.m_address = address;
-	}
-
-	link_address_map(symbol);
-	link_name_map(symbol);
-
-	return &symbol;
-}
-
 template <typename SymbolType>
 bool SymbolList<SymbolType>::move_symbol(SymbolHandle<SymbolType> handle, Address new_address)
 {
@@ -724,8 +731,8 @@ Result<DataType*> SymbolDatabase::create_data_type_if_unique(
 			}
 		}
 		if(!match) {
-			// This type doesn't match the others with the same name that have
-			// already been processed.
+			// This type doesn't match any of the others with the same name
+			// that have already been processed.
 			Result<DataType*> data_type = data_types.create_symbol(name, source, module_symbol);
 			CCC_RETURN_IF_ERROR(data_type);
 

src/ccc/symbol_database.h

@@ -171,6 +171,16 @@ class SymbolList {
 	// Retrieve the number of symbols stored.
 	s32 size() const;
 
+	// Create a new symbol. If it's a SymbolSource symbol, source can be left
+	// empty, otherwise it has to be valid.
+	Result<SymbolType*> create_symbol(
+		std::string name, Address address, SymbolSourceHandle source, const Module* module_symbol = nullptr);
+
+	// Create a new symbol. Similar to the above, but for symbols that don't
+	// have addresses.
+	Result<SymbolType*> create_symbol(
+		std::string name, SymbolSourceHandle source, const Module* module_symbol = nullptr);
+
 	// Create a new symbol if it doesn't already exist or
 	// DONT_DEDUPLICATE_SYMBOLS is set. If it's a SymbolSource symbol, source
 	// can be left empty, otherwise it has to be valid. The return value may
@@ -183,11 +193,6 @@ class SymbolList {
 		u32 importer_flags,
 		DemanglerFunctions demangler);
 
-	// Create a new symbol. If it's a SymbolSource symbol, source can be left
-	// empty, otherwise it has to be valid.
-	Result<SymbolType*> create_symbol(
-		std::string name, SymbolSourceHandle source, const Module* module_symbol = nullptr, Address address = Address());
-
 	// Update the address of a symbol without changing its handle.
 	bool move_symbol(SymbolHandle<SymbolType> handle, Address new_address);
 

src/ccc/symbol_table.h

@@ -85,6 +85,7 @@ class MdebugSymbolTable : public SymbolTable {
 		u32 importer_flags,
 		DemanglerFunctions demangler,
 		const std::atomic_bool* interrupt) const override;
+
 	Result<void> print_headers(FILE* out) const override;
 	Result<void> print_symbols(FILE* out, bool print_locals, bool print_externals) const override;
 

test/ccc/symbol_database_tests.cpp

@@ -90,7 +90,7 @@ TEST(CCCSymbolDatabase, HandlesFromAddressRange)
 
 	// Create the symbols.
 	for(u32 address = 10; address < 20; address++) {
-		Result<Function*> function = database.functions.create_symbol("", (*source)->handle(), nullptr, address);
+		Result<Function*> function = database.functions.create_symbol("", address, (*source)->handle(), nullptr);
 		CCC_GTEST_FAIL_IF_ERROR(function);
 		handles[address] = (*function)->handle();
 	}
@@ -137,7 +137,7 @@ TEST(CCCSymbolDatabase, HandleFromStartingAddress)
 
 	// Create the symbols.
 	for(u32 address = 0; address < 10; address++) {
-		Result<Function*> function = database.functions.create_symbol("", (*source)->handle(), nullptr, address);
+		Result<Function*> function = database.functions.create_symbol("", address, (*source)->handle(), nullptr);
 		CCC_GTEST_FAIL_IF_ERROR(function);
 		handles[address] = (*function)->handle();
 	}
@@ -184,7 +184,7 @@ TEST(CCCSymbolDatabase, HandlesFromName)
 
 static Result<FunctionHandle> create_function(SymbolDatabase& database, SymbolSourceHandle source, const char* name, Address address, u32 size)
 {
-	Result<Function*> function = database.functions.create_symbol("a", source, nullptr, address);
+	Result<Function*> function = database.functions.create_symbol("a", address, source, nullptr);
 	CCC_RETURN_IF_ERROR(function);
 	CCC_CHECK(*function, "*function");
 	(*function)->set_size(size);
@@ -265,7 +265,7 @@ TEST(CCCSymbolDatabase, MoveSymbol)
 	Result<SymbolSource*> source = database.symbol_sources.create_symbol("Source", SymbolSourceHandle());
 	CCC_GTEST_FAIL_IF_ERROR(source);
 
-	Result<Function*> function = database.functions.create_symbol("func", (*source)->handle(), nullptr, 0x1000);
+	Result<Function*> function = database.functions.create_symbol("func", 0x1000, (*source)->handle(), nullptr);
 	CCC_GTEST_FAIL_IF_ERROR(function);
 
 	EXPECT_TRUE(database.functions.move_symbol((*function)->handle(), 0x2000));
@@ -280,7 +280,7 @@ TEST(CCCSymbolDatabase, RenameSymbol)
 	Result<SymbolSource*> source = database.symbol_sources.create_symbol("Source", SymbolSourceHandle());
 	CCC_GTEST_FAIL_IF_ERROR(source);
 
-	Result<DataType*> data_type = database.data_types.create_symbol("Type1", (*source)->handle(), nullptr, 0x1000);
+	Result<DataType*> data_type = database.data_types.create_symbol("Type1", 0x1000, (*source)->handle(), nullptr);
 	EXPECT_TRUE(database.data_types.rename_symbol((*data_type)->handle(), "Type2"));
 
 	auto old_handles = database.data_types.handles_from_name("Type1");