From 8e1e97cccab4da8907cf9403e2c13514a4c285d3 Mon Sep 17 00:00:00 2001 From: Braulio Valdivielso Martinez Date: Mon, 7 Feb 2022 09:05:59 -0500 Subject: Trace: include `line_end` field in json-v1 format After !6954 got merged, it has become easier for tools to get full stack-traces for runtime traces of a CMake program. The trace information already included in the JSON objects (line number, source file path) allows tools that display these stack traces to print the CMake source code associated to them. However, CMake commands may spawn multiple lines, and the JSON information associated to a trace only contains the line in which the command started, but not the one in which it ended. If tools want to print stack traces along the relevant source code, and they want to print the whole command associated to the stack frame, they will have to implement their own CMake language parser to know where the command ends. In order to simplify the life of those who want to write tooling for CMake, this commit adds a `line_end` field to the json-v1 trace format. If a given command spans multiple lines, the `line_end` field will contain the line of the last line spanned by the command (that of the closing parenthesis associated to the command). --- Help/manual/cmake.1.rst | 8 ++++- Help/release/dev/trace-line-end.rst | 7 +++++ Source/cmCMakeLanguageCommand.cxx | 3 +- Source/cmCPluginAPI.cxx | 2 +- Source/cmListFileCache.cxx | 4 ++- Source/cmListFileCache.h | 9 ++++-- Source/cmMacroCommand.cxx | 2 +- Source/cmMakefile.cxx | 4 +++ Source/cmVariableWatchCommand.cxx | 2 +- Tests/RunCMake/CommandLine/trace-json-v1-check.py | 37 +++++++++++++++++------ Tests/RunCMake/CommandLine/trace-json-v1.cmake | 6 +++- 11 files changed, 64 insertions(+), 20 deletions(-) create mode 100644 Help/release/dev/trace-line-end.rst diff --git a/Help/manual/cmake.1.rst b/Help/manual/cmake.1.rst index 54eb244..37cf7d8 100644 --- a/Help/manual/cmake.1.rst +++ b/Help/manual/cmake.1.rst @@ -309,7 +309,13 @@ Options was called. ``line`` - The line in ``file`` of the function call. + The line in ``file`` where the function call begins. + + ``line_end`` + If the function call spans multiple lines, this field will + be set to the line where the function call ends. If the function + calls spans a single line, this field will be unset. This field + was added in minor version 2 of the ``json-v1`` format. ``defer`` Optional member that is present when the function call was deferred diff --git a/Help/release/dev/trace-line-end.rst b/Help/release/dev/trace-line-end.rst new file mode 100644 index 0000000..beade4b --- /dev/null +++ b/Help/release/dev/trace-line-end.rst @@ -0,0 +1,7 @@ +trace-line-end +-------------- + +* Add the field ``line_end`` to the json-v1 trace format. This + field tells you the line in file ``file`` at which the function + call ends. Tools can use this new field, together with ``line`` + and ``file``, to map traces to lines of CMake source code. diff --git a/Source/cmCMakeLanguageCommand.cxx b/Source/cmCMakeLanguageCommand.cxx index 789c78d..27d8cb8 100644 --- a/Source/cmCMakeLanguageCommand.cxx +++ b/Source/cmCMakeLanguageCommand.cxx @@ -84,7 +84,8 @@ bool cmCMakeLanguageCommandCALL(std::vector const& args, for (size_t i = startArg; i < args.size(); ++i) { funcArgs.emplace_back(args[i].Value, args[i].Delim, context.Line); } - cmListFileFunction func{ callCommand, context.Line, std::move(funcArgs) }; + cmListFileFunction func{ callCommand, context.Line, context.Line, + std::move(funcArgs) }; if (defer) { if (defer->Id.empty()) { diff --git a/Source/cmCPluginAPI.cxx b/Source/cmCPluginAPI.cxx index 1b11f20..abec968 100644 --- a/Source/cmCPluginAPI.cxx +++ b/Source/cmCPluginAPI.cxx @@ -432,7 +432,7 @@ static int CCONV cmExecuteCommand(void* arg, const char* name, int numArgs, lffArgs.emplace_back(args[i], cmListFileArgument::Quoted, 0); } - cmListFileFunction lff{ name, 0, std::move(lffArgs) }; + cmListFileFunction lff{ name, 0, 0, std::move(lffArgs) }; cmExecutionStatus status(*mf); return mf->ExecuteCommand(lff, status); } diff --git a/Source/cmListFileCache.cxx b/Source/cmListFileCache.cxx index b28b282..b90af08 100644 --- a/Source/cmListFileCache.cxx +++ b/Source/cmListFileCache.cxx @@ -40,6 +40,7 @@ struct cmListFileParser cmListFileLexer* Lexer; std::string FunctionName; long FunctionLine; + long FunctionLineEnd; std::vector FunctionArguments; enum { @@ -146,7 +147,7 @@ bool cmListFileParser::Parse() if (this->ParseFunction(token->text, token->line)) { this->ListFile->Functions.emplace_back( std::move(this->FunctionName), this->FunctionLine, - std::move(this->FunctionArguments)); + this->FunctionLineEnd, std::move(this->FunctionArguments)); } else { return false; } @@ -259,6 +260,7 @@ bool cmListFileParser::ParseFunction(const char* name, long line) } } else if (token->type == cmListFileLexer_Token_ParenRight) { if (parenDepth == 0) { + this->FunctionLineEnd = token->line; return true; } parenDepth--; diff --git a/Source/cmListFileCache.h b/Source/cmListFileCache.h index 58bc57e..c3da81b 100644 --- a/Source/cmListFileCache.h +++ b/Source/cmListFileCache.h @@ -51,9 +51,9 @@ struct cmListFileArgument class cmListFileFunction { public: - cmListFileFunction(std::string name, long line, + cmListFileFunction(std::string name, long line, long lineEnd, std::vector args) - : Impl{ std::make_shared(std::move(name), line, + : Impl{ std::make_shared(std::move(name), line, lineEnd, std::move(args)) } { } @@ -69,6 +69,7 @@ public: } long Line() const noexcept { return this->Impl->Line; } + long LineEnd() const noexcept { return this->Impl->LineEnd; } std::vector const& Arguments() const noexcept { @@ -78,11 +79,12 @@ public: private: struct Implementation { - Implementation(std::string name, long line, + Implementation(std::string name, long line, long lineEnd, std::vector args) : OriginalName{ std::move(name) } , LowerCaseName{ cmSystemTools::LowerCase(this->OriginalName) } , Line{ line } + , LineEnd{ lineEnd } , Arguments{ std::move(args) } { } @@ -90,6 +92,7 @@ private: std::string OriginalName; std::string LowerCaseName; long Line = 0; + long LineEnd = 0; std::vector Arguments; }; diff --git a/Source/cmMacroCommand.cxx b/Source/cmMacroCommand.cxx index 154df63..ef12487 100644 --- a/Source/cmMacroCommand.cxx +++ b/Source/cmMacroCommand.cxx @@ -116,7 +116,7 @@ bool cmMacroHelperCommand::operator()( newLFFArgs.push_back(std::move(arg)); } cmListFileFunction newLFF{ func.OriginalName(), func.Line(), - std::move(newLFFArgs) }; + func.LineEnd(), std::move(newLFFArgs) }; cmExecutionStatus status(makefile); if (!makefile.ExecuteCommand(newLFF, status) || status.GetNestedError()) { // The error message should have already included the call stack diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 934a6f4..77dbcfb 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -291,6 +291,9 @@ void cmMakefile::PrintCommandTrace( builder["indentation"] = ""; val["file"] = full_path; val["line"] = static_cast(lff.Line()); + if (lff.Line() != lff.LineEnd()) { + val["line_end"] = static_cast(lff.LineEnd()); + } if (deferId) { val["defer"] = *deferId; } @@ -1665,6 +1668,7 @@ void cmMakefile::Configure() this->Backtrace); cmListFileFunction project{ "project", 0, + 0, { { "Project", cmListFileArgument::Unquoted, 0 }, { "__CMAKE_INJECTED_PROJECT_COMMAND__", diff --git a/Source/cmVariableWatchCommand.cxx b/Source/cmVariableWatchCommand.cxx index fd5402c..24394d9 100644 --- a/Source/cmVariableWatchCommand.cxx +++ b/Source/cmVariableWatchCommand.cxx @@ -58,7 +58,7 @@ void cmVariableWatchCommandVariableAccessed(const std::string& variable, { stack, cmListFileArgument::Quoted, fakeLineNo } }; - cmListFileFunction newLFF{ data->Command, fakeLineNo, + cmListFileFunction newLFF{ data->Command, fakeLineNo, fakeLineNo, std::move(newLFFArgs) }; cmExecutionStatus status(*makefile); if (!makefile->ExecuteCommand(newLFF, status)) { diff --git a/Tests/RunCMake/CommandLine/trace-json-v1-check.py b/Tests/RunCMake/CommandLine/trace-json-v1-check.py index 995cfad..2ef1495 100755 --- a/Tests/RunCMake/CommandLine/trace-json-v1-check.py +++ b/Tests/RunCMake/CommandLine/trace-json-v1-check.py @@ -30,6 +30,8 @@ required_traces = [ { 'args': ['STATUS', 'JSON-V1 str', 'spaces'], 'cmd': 'message', + 'line': 1, + 'line_end': 5 }, { 'args': ['ASDF', 'fff', 'sss', ' SPACES !!! '], @@ -57,6 +59,24 @@ required_traces = [ } ] +def assert_fields_look_good(line): + expected_fields = {'args', 'cmd', 'file', 'frame', 'global_frame','line', 'time'} + if "line_end" in line: + assert isinstance(line['line_end'], int) + assert line['line'] != line['line_end'] + expected_fields.add("line_end") + + assert set(line.keys()) == expected_fields + + assert isinstance(line['args'], list) + assert isinstance(line['cmd'], unicode) + assert isinstance(line['file'], unicode) + assert isinstance(line['frame'], int) + assert isinstance(line['global_frame'], int) + assert isinstance(line['line'], int) + assert isinstance(line['time'], float) + + with open(trace_file, 'r') as fp: # Check for version (must be the first document) vers = json.loads(fp.readline()) @@ -67,18 +87,15 @@ with open(trace_file, 'r') as fp: for i in fp.readlines(): line = json.loads(i) - assert sorted(line.keys()) == ['args', 'cmd', 'file', 'frame', 'global_frame','line', 'time'] - assert isinstance(line['args'], list) - assert isinstance(line['cmd'], unicode) - assert isinstance(line['file'], unicode) - assert isinstance(line['frame'], int) - assert isinstance(line['global_frame'], int) - assert isinstance(line['line'], int) - assert isinstance(line['time'], float) - + assert_fields_look_good(line) for j in required_traces: # Compare the subset of required keys with line - if {k: line[k] for k in j} == j: + subset = { + k: line[k] + for k in j + if k in line + } + if subset == j: required_traces.remove(j) assert not required_traces diff --git a/Tests/RunCMake/CommandLine/trace-json-v1.cmake b/Tests/RunCMake/CommandLine/trace-json-v1.cmake index 871746d..4ed6160 100644 --- a/Tests/RunCMake/CommandLine/trace-json-v1.cmake +++ b/Tests/RunCMake/CommandLine/trace-json-v1.cmake @@ -1,4 +1,8 @@ -message(STATUS "JSON-V1 str" "spaces") +message( + STATUS + "JSON-V1 str" + "spaces" + ) set(ASDF fff sss " SPACES !!! ") set(FOO 42) set(BAR " space in string!") -- cgit v0.12