This is an archive of the discontinued LLVM Phabricator instance.

@labath, since you committed D23882, could you, please, take a look at this one as well? It's immediately following that, and should fix the build issue resulting from use of private headers.

krytarowski accepted this revision.Sep 10 2016, 2:38 PM

krytarowski added a reviewer: krytarowski.

This revision is now accepted and ready to land.Sep 10 2016, 2:38 PM

It looks good. Remaining TODO for standalone builds:

sanitize six.py usage (it's installed into system-wide directory, where standard py-six lands)
bump cmake_minimum_required(VERSION 2.8.12.2) to 3.4.3 in cmake/modules/LLDBStandalone.cmake

Final bits for Windows:

eliminate the usage of ../lib/Support/regex_impl.h in RegularExpression.h

In D23883#539442, @krytarowski wrote:

It looks good. Remaining TODO for standalone builds:

sanitize six.py usage (it's installed into system-wide directory, where standard py-six lands)

How would you prefer handling it? Can we just kill it and rely on user installing it system-wide or via pip?

bump cmake_minimum_required(VERSION 2.8.12.2) to 3.4.3 in cmake/modules/LLDBStandalone.cmake

Final bits for Windows:

eliminate the usage of ../lib/Support/regex_impl.h in RegularExpression.h

Wouldn't it be actually better to kill that regex implementation as well, and use the class provided by LLVMSupport everywhere? I started with the other since it caused direct build issues on Linux but I think I could try to kill this one as well.

In D23883#539471, @mgorny wrote:

In D23883#539442, @krytarowski wrote:

It looks good. Remaining TODO for standalone builds:

sanitize six.py usage (it's installed into system-wide directory, where standard py-six lands)

How would you prefer handling it? Can we just kill it and rely on user installing it system-wide or via pip?

Patch scripts/Python/finishSwigPythonLLDB.py and install six into lldb subdir with a custom name (lldb_six.py).
Patch the Python sources to make use of lldb_six.

Unfortunately there is resistance to grab system-wide (provided by a user) six module.

bump cmake_minimum_required(VERSION 2.8.12.2) to 3.4.3 in cmake/modules/LLDBStandalone.cmake

Final bits for Windows:

eliminate the usage of ../lib/Support/regex_impl.h in RegularExpression.h

Wouldn't it be actually better to kill that regex implementation as well, and use the class provided by LLVMSupport everywhere? I started with the other since it caused direct build issues on Linux but I think I could try to kill this one as well.

It makes sense.

You forgot to remove its header file

This revision now requires changes to proceed.Sep 11 2016, 10:37 PM

mgorny updated this revision to Diff 70997.Sep 12 2016, 5:32 AM

mgorny edited edge metadata.

Herald added subscribers: mgorny, beanz. · View Herald TranscriptSep 12 2016, 5:32 AM

In D23883#539611, @ki.stfu wrote:

You forgot to remove its header file

Thanks for noticing. Fixed now.

@mgorny: The change should be reviewed by an lldb-mi maintainer (i.e., @ki.stfu). It helps is you explicitly specify the reviewer, as otherwise the person may not notice the patch.

In D23883#539471, @mgorny wrote:

In D23883#539442, @krytarowski wrote:

It looks good. Remaining TODO for standalone builds:

sanitize six.py usage (it's installed into system-wide directory, where standard py-six lands)

How would you prefer handling it? Can we just kill it and rely on user installing it system-wide or via pip?

I think the most politically passable version would be to make that a cmake option (LLDB_USE_BUILTIN_SIX ?). If it is set you use the system-wide package, otherwise, you use the builtin one. When you build the distro package, you set it to false. When e.g., we are distributing it with Android Studio, we'll set it to true. I personally don't care what the default will be.

bump cmake_minimum_required(VERSION 2.8.12.2) to 3.4.3 in cmake/modules/LLDBStandalone.cmake

Final bits for Windows:

eliminate the usage of ../lib/Support/regex_impl.h in RegularExpression.h

Wouldn't it be actually better to kill that regex implementation as well, and use the class provided by LLVMSupport everywhere? I started with the other since it caused direct build issues on Linux but I think I could try to kill this one as well.

I think that would be great. We'll need to be a bit careful and check whether we the two libraries have sufficiently similar interfaces.

lgtm

This revision is now accepted and ready to land.Sep 12 2016, 11:50 AM

I am going to assume you still want me to submit this. r281317.

Yes, thanks a lot.

Revision Contents

Path

Size

tools/

lldb-mi/

CMakeLists.txt

1 line

MIUtilParse.h

MIUtilParse.cpp

Diff 70997

tools/lldb-mi/CMakeLists.txt

Show First 20 Lines • Show All 59 Lines • ▼ Show 20 Lines	set(LLDB_MI_SOURCES
MICmnStreamStderr.cpp		MICmnStreamStderr.cpp
MICmnStreamStdin.cpp		MICmnStreamStdin.cpp
MICmnStreamStdout.cpp		MICmnStreamStdout.cpp
MICmnThreadMgrStd.cpp		MICmnThreadMgrStd.cpp
MIDriver.cpp		MIDriver.cpp
MIDriverBase.cpp		MIDriverBase.cpp
MIDriverMain.cpp		MIDriverMain.cpp
MIDriverMgr.cpp		MIDriverMgr.cpp
MIUtilParse.cpp
MIUtilDateTimeStd.cpp		MIUtilDateTimeStd.cpp
MIUtilDebug.cpp		MIUtilDebug.cpp
MIUtilFileStd.cpp		MIUtilFileStd.cpp
MIUtilMapIdToVariant.cpp		MIUtilMapIdToVariant.cpp
MIUtilString.cpp		MIUtilString.cpp
MIUtilThreadBaseStd.cpp		MIUtilThreadBaseStd.cpp
MIUtilVariant.cpp		MIUtilVariant.cpp
)		)
Show All 21 Lines

tools/lldb-mi/MIUtilParse.h

This file was deleted.

	//===-- MIUtilParse.h ------------------------------------------- C++ --===//
	//
	// The LLVM Compiler Infrastructure
	//
	// This file is distributed under the University of Illinois Open Source
	// License. See LICENSE.TXT for details.
	//
	//===----------------------------------------------------------------------===//

	#pragma once

	// Third party headers:
	#include "../lib/Support/regex_impl.h"

	// In-house headers:
	#include "MIUtilString.h"

	namespace MIUtilParse {

	//++
	//============================================================================
	// Details: MI common code utility class. Used to parse the output
	// returned from lldb commands using regex.
	//--
	class CRegexParser {
	public:
	// Helper class for keeping track of regex matches.
	class Match {
	friend CRegexParser;

	public:
	/* ctor */ explicit Match(size_t nmatches)
	: m_matchStrs(nmatches), m_maxMatches(nmatches) {}
	size_t GetMatchCount() const { return m_matchStrs.size(); }
	CMIUtilString GetMatchAtIndex(size_t i) const {
	if (m_matchStrs.size() > i)
	return m_matchStrs[i];
	return CMIUtilString();
	}

	private:
	CMIUtilString::VecString_t m_matchStrs;
	const size_t m_maxMatches;
	};

	// Methods:
	// Compile the regular expression.
	/* ctor / explicit CRegexParser(const char regexStr);

	// Free the memory used by the regular expression.
	/* dtor */ ~CRegexParser();

	// No copies
	CRegexParser(const CRegexParser &) = delete;
	void operator=(CRegexParser &) = delete;

	// Return the match at the index.
	int GetMatchCount(const Match &match) const {
	if (m_isValid)
	return match.GetMatchCount();
	return 0;
	}

	bool IsValid() const { return m_isValid; }

	// Match the input against the regular expression. Return an error
	// if the number of matches is less than minMatches. If the default
	// minMatches value of 0 is passed, an error will be returned if
	// the number of matches is less than the maxMatches value used to
	// initialize Match.
	bool Execute(const char *input, Match &match, size_t minMatches = 0);

	private:
	llvm_regex_t m_emma;
	const bool m_isValid;
	};
	}

tools/lldb-mi/MIUtilParse.cpp

This file was deleted.

	//===-- MIUtilParse.cpp ----------------------------------------- C++ --===//
	//
	// The LLVM Compiler Infrastructure
	//
	// This file is distributed under the University of Illinois Open Source
	// License. See LICENSE.TXT for details.
	//
	//===----------------------------------------------------------------------===//

	// Third party headers:
	#include <memory>

	// In-house headers:
	#include "MIUtilParse.h"

	//++
	//------------------------------------------------------------------------------------
	// Details: CRegexParser constructor.
	// Type: Method.
	// Args: regexStr - Pointer to the regular expression to compile.
	// Return: None.
	// Throws: None.
	//--
	MIUtilParse::CRegexParser::CRegexParser(const char *regexStr)
	: m_isValid(llvm_regcomp(&m_emma, regexStr, REG_EXTENDED) == 0) {}

	//++
	//------------------------------------------------------------------------------------
	// Details: CRegexParser destructor.
	// Type: Method.
	// Args: None.
	// Return: None.
	// Throws: None.
	//--
	MIUtilParse::CRegexParser::~CRegexParser() {
	// Free up memory held within regex.
	if (m_isValid)
	llvm_regfree(&m_emma);
	}

	//++
	//------------------------------------------------------------------------------------
	// Details: CRegexParser regex executer.
	// Match the input against the regular expression. Return an error
	// if the number of matches is less than minMatches. If the default
	// minMatches value of 0 is passed, an error will be returned if
	// the number of matches is less than the maxMatches value used to
	// initialize Match.
	// Type: Method.
	// Args: input (R) - Pointer to UTF8 text data to be parsed.
	// match (RW) - Reference to Match class.
	// minMatches (R) - Minimum number of regex matches expected.
	// Return: bool - True = minimum matches were met,
	// false = minimum matches were not met or regex failed.
	// Throws: None.
	//--
	bool MIUtilParse::CRegexParser::Execute(const char *input, Match &match,
	size_t minMatches) {
	if (!m_isValid)
	return false;

	std::unique_ptr<llvm_regmatch_t[]> matches(
	new llvm_regmatch_t[match.m_maxMatches]); // Array of matches

	if (llvm_regexec(&m_emma, input, match.m_maxMatches, matches.get(), 0) != 0)
	return false;

	size_t i;
	for (i = 0; i < match.m_maxMatches && matches[i].rm_so >= 0; i++) {
	const int n = matches[i].rm_eo - matches[i].rm_so;
	match.m_matchStrs[i].assign(input + matches[i].rm_so, n);
	}
	return i >= minMatches;
	}

This is an archive of the discontinued LLVM Phabricator instance.

Remove MIUtilParse (no longer used)ClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents

Diff 70997

tools/lldb-mi/CMakeLists.txt

tools/lldb-mi/MIUtilParse.h

tools/lldb-mi/MIUtilParse.cpp

Remove MIUtilParse (no longer used)
ClosedPublic