Merge pull request #15309 from ethereum/fix-wrong-native-locations-in…

…-optimized-yul-artifacts

Fix wrong native locations in optimized Yul artifacts

Changelog.md

@@ -30,8 +30,9 @@ Bugfixes:
  * SMTChecker: Fix internal compiler error when reporting proved targets for BMC engine.
  * SMTChecker: Fix SMT logic error when assigning to an array of contracts or functions.
  * TypeChecker: Fix segfault when assigning nested tuple to tuple.
- * Yul AST: Fix ``nativeSrc`` attributes in optimized IR AST referring to locations in unoptimized IR.
  * Yul IR Code Generation: Deterministic order of Yul subobjects.
+ * Yul Optimizer: Fix Yul source locations always referring to unoptimized source, even in optimized outputs.
+ * Yul Optimizer: Fix warnings being generated twice when there are no errors.
  * Yul Optimizer: Name simplification could lead to forbidden identifiers with a leading and/or trailing dot, e.g., ``x._`` would get simplified into ``x.``.
  * Yul Parser: Fix segfault when parsing very long location comments.
 

libsolidity/interface/CompilerStack.cpp

@@ -1488,39 +1488,26 @@ void CompilerStack::generateIR(ContractDefinition const& _contract, bool _unopti
 		);
 	}
 
-	auto const parseYul = [&](std::string const& _irSource) {
-		YulStack stack(
-			m_evmVersion,
-			m_eofVersion,
-			YulStack::Language::StrictAssembly,
-			m_optimiserSettings,
-			m_debugInfoSelection
-		);
-		bool yulAnalysisSuccessful = stack.parseAndAnalyze("", _irSource);
-		solAssert(
-			yulAnalysisSuccessful,
-			_irSource + "\n\n"
-			"Invalid IR generated:\n" +
-			langutil::SourceReferenceFormatter::formatErrorInformation(stack.errors(), stack) + "\n"
-		);
-		return stack;
-	};
-
-	{
-		YulStack stack = parseYul(compiledContract.yulIR);
-		compiledContract.yulIRAst = stack.astJson();
-		if (!_unoptimizedOnly)
-		{
-			stack.optimize();
-			compiledContract.yulIROptimized = stack.print(this);
-		}
-	}
+	YulStack stack(
+		m_evmVersion,
+		m_eofVersion,
+		YulStack::Language::StrictAssembly,
+		m_optimiserSettings,
+		m_debugInfoSelection
+	);
+	bool yulAnalysisSuccessful = stack.parseAndAnalyze("", compiledContract.yulIR);
+	solAssert(
+		yulAnalysisSuccessful,
+		compiledContract.yulIR + "\n\n"
+		"Invalid IR generated:\n" +
+		SourceReferenceFormatter::formatErrorInformation(stack.errors(), stack) + "\n"
+	);
 
+	compiledContract.yulIRAst = stack.astJson();
 	if (!_unoptimizedOnly)
 	{
-		// Optimizer does not maintain correct native source locations in the AST.
-		// We can work around it by regenerating the AST from scratch from optimized IR.
-		YulStack stack = parseYul(compiledContract.yulIROptimized);
+		stack.optimize();
+		compiledContract.yulIROptimized = stack.print(this);
 		compiledContract.yulIROptimizedAst = stack.astJson();
 	}
 }

libyul/YulStack.cpp

@@ -30,6 +30,7 @@
 #include <libyul/optimiser/Suite.h>
 #include <libevmasm/Assembly.h>
 #include <liblangutil/Scanner.h>
+#include <liblangutil/SourceReferenceFormatter.h>
 #include <libsolidity/interface/OptimiserSettings.h>
 
 #include <boost/algorithm/string.hpp>
@@ -117,7 +118,10 @@ void YulStack::optimize()
 
 		m_stackState = Parsed;
 		optimize(*m_parserResult, true);
-		yulAssert(analyzeParsed(), "Invalid source code after optimization.");
+
+		// Optimizer does not maintain correct native source locations in the AST.
+		// We can work around it by regenerating the AST from scratch from optimized IR.
+		reparse();
 	}
 	catch (UnimplementedFeatureError const& _error)
 	{
@@ -241,6 +245,33 @@ void YulStack::optimize(Object& _object, bool _isCreation)
 	);
 }
 
+void YulStack::reparse()
+{
+	yulAssert(m_parserResult);
+	yulAssert(m_charStream);
+
+	// NOTE: Without passing in _soliditySourceProvider, printed debug info will not include code
+	// snippets, but it does not matter - we'll still get the same AST after we parse it. Snippets
+	// are not stored in the AST and the other info that is (location, AST ID, etc) will still be present.
+	std::string source = print(nullptr /* _soliditySourceProvider */);
+
+	YulStack cleanStack(m_evmVersion, m_eofVersion, m_language, m_optimiserSettings, m_debugInfoSelection);
+	bool reanalysisSuccessful = cleanStack.parseAndAnalyze(m_charStream->name(), source);
+	yulAssert(
+		reanalysisSuccessful,
+		source + "\n\n"
+		"Invalid IR generated:\n" +
+		SourceReferenceFormatter::formatErrorInformation(cleanStack.errors(), cleanStack) + "\n"
+	);
+
+	m_stackState = AnalysisSuccessful;
+	m_parserResult = std::move(cleanStack.m_parserResult);
+
+	// NOTE: We keep the char stream, and errors, even though they no longer match the object,
+	// because it's the original source that matters to the user. Optimized code may have different
+	// locations and fewer warnings.
+}
+
 MachineAssemblyObject YulStack::assemble(Machine _machine)
 {
 	yulAssert(m_stackState >= AnalysisSuccessful);

libyul/YulStack.h

@@ -152,6 +152,13 @@ class YulStack: public langutil::CharStreamProvider
 
 	void optimize(yul::Object& _object, bool _isCreation);
 
+	/// Prints the Yul object stored internally and parses it again.
+	/// This ensures that the debug info in the AST matches the source that printing would produce
+	/// rather than the initial source.
+	/// @warning Does not update the error list or the original source (@a m_charStream) to make
+	/// it possible to report errors to the user even after the optimization has been performed.
+	void reparse();
+
 	void reportUnimplementedFeatureError(langutil::UnimplementedFeatureError const& _error);
 
 	Language m_language = Language::Assembly;

libyul/backends/evm/EVMObjectCompiler.cpp

@@ -35,7 +35,7 @@
 using namespace solidity::yul;
 
 void EVMObjectCompiler::compile(
-	Object& _object,
+	Object const& _object,
 	AbstractAssembly& _assembly,
 	EVMDialect const& _dialect,
 	bool _optimize,
@@ -46,7 +46,7 @@ void EVMObjectCompiler::compile(
 	compiler.run(_object, _optimize);
 }
 
-void EVMObjectCompiler::run(Object& _object, bool _optimize)
+void EVMObjectCompiler::run(Object const& _object, bool _optimize)
 {
 	BuiltinContext context;
 	context.currentObject = &_object;

libyul/backends/evm/EVMObjectCompiler.h

@@ -34,7 +34,7 @@ class EVMObjectCompiler
 {
 public:
 	static void compile(
-		Object& _object,
+		Object const& _object,
 		AbstractAssembly& _assembly,
 		EVMDialect const& _dialect,
 		bool _optimize,
@@ -45,7 +45,7 @@ class EVMObjectCompiler
 		m_assembly(_assembly), m_dialect(_dialect), m_eofVersion(_eofVersion)
 	{}
 
-	void run(Object& _object, bool _optimize);
+	void run(Object const& _object, bool _optimize);
 
 	AbstractAssembly& m_assembly;
 	EVMDialect const& m_dialect;