Merge pull request #8224 from a3d4/refactor-sourcelocation

Replaced SourceLocation::isEmpty() with isValid() and hasText().

libevmasm/Assembly.cpp

@@ -43,7 +43,7 @@ AssemblyItem const& Assembly::append(AssemblyItem const& _i)
 	assertThrow(m_deposit >= 0, AssemblyException, "Stack underflow.");
 	m_deposit += _i.deposit();
 	m_items.emplace_back(_i);
-	if (m_items.back().location().isEmpty() && !m_currentSourceLocation.isEmpty())
+	if (!m_items.back().location().isValid() && m_currentSourceLocation.isValid())
 		m_items.back().setLocation(m_currentSourceLocation);
 	m_items.back().m_modifierDepth = m_currentModifierDepth;
 	return m_items.back();
@@ -69,7 +69,7 @@ namespace
 
 string locationFromSources(StringMap const& _sourceCodes, SourceLocation const& _location)
 {
-	if (_location.isEmpty() || !_location.source.get() || _sourceCodes.empty() || _location.start >= _location.end || _location.start < 0)
+	if (!_location.hasText() || _sourceCodes.empty())
 		return "";
 
 	auto it = _sourceCodes.find(_location.source->name());
@@ -97,7 +97,7 @@ public:
 
 	void feed(AssemblyItem const& _item)
 	{
-		if (!_item.location().isEmpty() && _item.location() != m_location)
+		if (_item.location().isValid() && _item.location() != m_location)
 		{
 			flush();
 			m_location = _item.location();
@@ -141,12 +141,12 @@ public:
 
 	void printLocation()
 	{
-		if (!m_location.source && m_location.isEmpty())
+		if (!m_location.isValid())
 			return;
 		m_out << m_prefix << "    /*";
 		if (m_location.source)
 			m_out << " \"" + m_location.source->name() + "\"";
-		if (!m_location.isEmpty())
+		if (m_location.hasText())
 			m_out << ":" << to_string(m_location.start) + ":" + to_string(m_location.end);
 		m_out << "  " << locationFromSources(m_sourceCodes, m_location);
 		m_out << " */" << endl;

liblangutil/Exceptions.cpp

@@ -51,7 +51,7 @@ Error::Error(Type _type, SourceLocation const& _location, string const& _descrip
 		break;
 	}
 
-	if (!_location.isEmpty())
+	if (_location.isValid())
 		*this << errinfo_sourceLocation(_location);
 	if (!_description.empty())
 		*this << util::errinfo_comment(_description);
@@ -60,7 +60,7 @@ Error::Error(Type _type, SourceLocation const& _location, string const& _descrip
 Error::Error(Error::Type _type, std::string const& _description, SourceLocation const& _location):
 	Error(_type)
 {
-	if (!_location.isEmpty())
+	if (_location.isValid())
 		*this << errinfo_sourceLocation(_location);
 	*this << util::errinfo_comment(_description);
 }

liblangutil/Scanner.h

@@ -167,12 +167,6 @@ public:
 	/// Do only use in error cases, they are quite expensive.
 	std::string lineAtPosition(int _position) const { return m_source->lineAtPosition(_position); }
 	std::tuple<int, int> translatePositionToLineColumn(int _position) const { return m_source->translatePositionToLineColumn(_position); }
-	std::string sourceAt(SourceLocation const& _location) const
-	{
-		solAssert(!_location.isEmpty(), "");
-		solAssert(m_source.get() == _location.source.get(), "CharStream memory locations must match.");
-		return m_source->source().substr(_location.start, _location.end - _location.start);
-	}
 	///@}
 
 private:

liblangutil/SourceLocation.h

@@ -56,24 +56,33 @@ struct SourceLocation
 
 	inline bool contains(SourceLocation const& _other) const
 	{
-		if (isEmpty() || _other.isEmpty() || source.get() != _other.source.get())
+		if (!hasText() || !_other.hasText() || source.get() != _other.source.get())
 			return false;
 		return start <= _other.start && _other.end <= end;
 	}
 
 	inline bool intersects(SourceLocation const& _other) const
 	{
-		if (isEmpty() || _other.isEmpty() || source.get() != _other.source.get())
+		if (!hasText() || !_other.hasText() || source.get() != _other.source.get())
 			return false;
 		return _other.start < end && start < _other.end;
 	}
 
-	bool isEmpty() const { return start == -1 && end == -1; }
+	bool isValid() const { return source || start != -1 || end != -1; }
+
+	bool hasText() const
+	{
+		return
+			source &&
+			0 <= start &&
+			start <= end &&
+			end <= int(source->source().length());
+	}
 
 	std::string text() const
 	{
 		assertThrow(source, SourceLocationError, "Requested text from null source.");
-		assertThrow(!isEmpty(), SourceLocationError, "Requested text from empty source location.");
+		assertThrow(0 <= start, SourceLocationError, "Invalid source location.");
 		assertThrow(start <= end, SourceLocationError, "Invalid source location.");
 		assertThrow(end <= int(source->source().length()), SourceLocationError, "Invalid source location.");
 		return source->source().substr(start, end - start);
@@ -109,13 +118,13 @@ SourceLocation const parseSourceLocation(std::string const& _input, std::string
 /// Stream output for Location (used e.g. in boost exceptions).
 inline std::ostream& operator<<(std::ostream& _out, SourceLocation const& _location)
 {
-	if (_location.isEmpty())
+	if (!_location.isValid())
 		return _out << "NO_LOCATION_SPECIFIED";
 
 	if (_location.source)
 		_out << _location.source->name();
 
-	_out << "[" << _location.start << "," << _location.end << ")";
+	_out << "[" << _location.start << "," << _location.end << "]";
 
 	return _out;
 }

libsolidity/analysis/ControlFlowAnalyzer.cpp

@@ -163,7 +163,7 @@ void ControlFlowAnalyzer::checkUnreachable(CFGNode const* _entry, CFGNode const*
 	std::set<SourceLocation> unreachable;
 	util::BreadthFirstSearch<CFGNode const*>{{_exit, _revert}}.run(
 		[&](CFGNode const* _node, auto&& _addChild) {
-			if (!reachable.count(_node) && !_node->location.isEmpty())
+			if (!reachable.count(_node) && _node->location.isValid())
 				unreachable.insert(_node->location);
 			for (CFGNode const* entry: _node->entries)
 				_addChild(entry);

libsolidity/analysis/TypeChecker.cpp

@@ -2762,7 +2762,7 @@ bool TypeChecker::visit(Identifier const& _identifier)
 				SecondarySourceLocation ssl;
 
 				for (Declaration const* declaration: annotation.overloadedDeclarations)
-					if (declaration->location().isEmpty())
+					if (!declaration->location().isValid())
 					{
 						// Try to re-construct function definition
 						string description;

libsolidity/ast/AsmJsonImporter.cpp

@@ -54,9 +54,11 @@ T AsmJsonImporter::createAsmNode(Json::Value const& _node)
 {
 	T r;
 	r.location = createSourceLocation(_node);
-	astAssert(!r.location.isEmpty() || !r.location.source, "Invalid source location in Asm AST");
+	astAssert(
+		r.location.source && 0 <= r.location.start && r.location.start <= r.location.end,
+		"Invalid source location in Asm AST"
+	);
 	return r;
-
 }
 
 Json::Value AsmJsonImporter::member(Json::Value const& _node, string const& _name)

libyul/AsmParser.h

@@ -65,7 +65,7 @@ protected:
 	{
 		T r;
 		r.location = _loc;
-		if (r.location.isEmpty())
+		if (!r.location.hasText())
 		{
 			r.location.start = position();
 			r.location.end = endPosition();

test/libevmasm/Assembler.cpp

@@ -51,11 +51,11 @@ BOOST_AUTO_TEST_SUITE(Assembler)
 BOOST_AUTO_TEST_CASE(all_assembly_items)
 {
 	Assembly _assembly;
-	auto root_asm = make_shared<CharStream>("", "root.asm");
+	auto root_asm = make_shared<CharStream>("lorem ipsum", "root.asm");
 	_assembly.setSourceLocation({1, 3, root_asm});
 
 	Assembly _subAsm;
-	auto sub_asm = make_shared<CharStream>("", "sub.asm");
+	auto sub_asm = make_shared<CharStream>("lorem ipsum", "sub.asm");
 	_subAsm.setSourceLocation({6, 8, sub_asm});
 	_subAsm.append(Instruction::INVALID);
 	shared_ptr<Assembly> _subAsmPtr = make_shared<Assembly>(_subAsm);

test/libevmasm/Optimiser.cpp

@@ -72,7 +72,7 @@ namespace
 
 		for (AssemblyItem const& item: output)
 		{
-			BOOST_CHECK(item == Instruction::POP || !item.location().isEmpty());
+			BOOST_CHECK(item == Instruction::POP || item.location().isValid());
 		}
 		return output;
 	}

test/liblangutil/SourceLocation.cpp

@@ -33,9 +33,9 @@ BOOST_AUTO_TEST_SUITE(SourceLocationTest)
 
 BOOST_AUTO_TEST_CASE(test_fail)
 {
-	auto const source = std::make_shared<CharStream>("", "source");
+	auto const source = std::make_shared<CharStream>("lorem ipsum", "source");
-	auto const sourceA = std::make_shared<CharStream>("", "sourceA");
+	auto const sourceA = std::make_shared<CharStream>("lorem ipsum", "sourceA");
-	auto const sourceB = std::make_shared<CharStream>("", "sourceB");
+	auto const sourceB = std::make_shared<CharStream>("lorem ipsum", "sourceB");
 
 	BOOST_CHECK(SourceLocation{} == SourceLocation{});
 	BOOST_CHECK((SourceLocation{0, 3, sourceA} != SourceLocation{0, 3, sourceB}));