This is an archive of the discontinued LLVM Phabricator instance.

Remove Windows editline from LLDB
ClosedPublic

Authored by nealsid on May 10 2021, 7:25 PM.

Download Raw Diff

Details

Reviewers

kparzysz
stella.stamenova
amccarth

Group Reviewers

Restricted Project

Commits

rG5af3a6645f38: Remove Windows editline from LLDB

Summary

I don't mean to undo others' work but it looks like the hand-rolled EditLine for LLDB on Windows isn't used. It'd be easier to make changes to bring the other platforms' Editline wrapper up to date (e.g. simplifying char vs wchar_t) without modifying/testing this one too.

Diff Detail

Repository: rG LLVM Github Monorepo

Event Timeline

nealsid created this revision.May 10 2021, 7:25 PM

Herald added subscribers: hiraditya, krytarowski, mgorny. · View Herald TranscriptMay 10 2021, 7:25 PM

nealsid requested review of this revision.May 10 2021, 7:25 PM

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 10 2021, 7:25 PM

Herald added subscribers: llvm-commits, lldb-commits. · View Herald Transcript

Harbormaster completed remote builds in B103647: Diff 344270.May 10 2021, 8:07 PM

Herald added a subscriber: JDevlieghere. · View Herald TranscriptMay 10 2021, 8:07 PM

DavidSpickett added a subscriber: DavidSpickett.May 11 2021, 2:44 AM

DavidSpickett added inline comments.

llvm/lib/Target/Hexagon/HexagonVectorCombine.cpp
302 ↗	(On Diff #344270)	This should be done in a separate change.

No idea about how/if this is used, but I think Stella or Adrian are the ones that might be able to answer this.

It looks like this EditLine port was added before I joined this project. @zturner may have worked on it, but I don't know/remember.

If it's actually unused, I have no objection to removing it. I harbor some hope that Windows's steadily improving unix-like terminal implementation (which console programs can enable) will eventually make a Windows-port of EditLine trivial.

I concur that the MSVC version tweak should be a separate change. If you remove that from this patch, I'll LGTM.

Updated the diff after moving an unrelated change into another revision.

nealsid edited the summary of this revision. (Show Details)May 11 2021, 11:03 AM

I don't have context into this either. Does this cause any test failures or is everything still green after the change?

I ran LLDB tests from a clean tree and with my change and the failures seemed to be the same, but there is a large amount of them. I also did manual testing with the changes. Build is still green.

Editline is only used when Cmake can find the editline libraries, which it never does on Windows. So the code that is actually used for console I/O is the code in the IOHandlerEditline class in IOHandler.cpp that is outside of the #ifdef LLDB_USE_EDITLINE preprocessor check. That uses POSIX functions for console I/O. I think it may be a good follow up change to make IOHandlerEditline be used in editline-specific cases and move the fallback/non-editline code in that class to another class.

Harbormaster completed remote builds in B103801: Diff 344489.May 11 2021, 12:00 PM

LGTM. Dead code should go. If somebody wants to own a Windows port of EditLine, they can re-instate it and put some tests on it.

Thanks.

This revision is now accepted and ready to land.May 11 2021, 3:02 PM

Thanks, all. If there are no more comments, could someone land it for me? I don't have commit access.

This revision was landed with ongoing or failed builds.May 12 2021, 2:05 AM

Closed by commit rG5af3a6645f38: Remove Windows editline from LLDB (authored by nealsid, committed by DavidSpickett). · Explain Why

This revision was automatically updated to reflect the committed changes.

DavidSpickett added a commit: rG5af3a6645f38: Remove Windows editline from LLDB.

Thanks, all. If there are no more comments, could someone land it for me? I don't have commit access.

Done.

Revision Contents

Path

Size

lldb/

include/

lldb/

Host/

Editline.h

4 lines

windows/

editlinewin.h

source/

Host/

CMakeLists.txt

1 line

windows/

EditLineWin.cpp

Diff 344725

lldb/include/lldb/Host/Editline.h

	Show All 34 Lines
	#include <codecvt>			#include <codecvt>
	#endif			#endif
	#include <locale>			#include <locale>
	#include <sstream>			#include <sstream>
	#include <vector>			#include <vector>

	#include "lldb/lldb-private.h"			#include "lldb/lldb-private.h"

	#if defined(_WIN32)			#if !defined(_WIN32) && !defined(__ANDROID__)
	#include "lldb/Host/windows/editlinewin.h"
	#elif !defined(__ANDROID__)
	#include <histedit.h>			#include <histedit.h>
	#endif			#endif

	#include <csignal>			#include <csignal>
	#include <mutex>			#include <mutex>
	#include <string>			#include <string>
	#include <vector>			#include <vector>

	▲ Show 20 Lines • Show All 349 Lines • Show Last 20 Lines

lldb/include/lldb/Host/windows/editlinewin.h

This file was deleted.

	//===-- editlinewin.h -------------------------------------------- C++ --===//
	//
	// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
	// See https://llvm.org/LICENSE.txt for license information.
	// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
	//
	//===----------------------------------------------------------------------===//

	#pragma once

	#include <stdio.h>

	// EditLine editor function return codes.
	// For user-defined function interface
	#define CC_NORM 0
	#define CC_NEWLINE 1
	#define CC_EOF 2
	#define CC_ARGHACK 3
	#define CC_REFRESH 4
	#define CC_CURSOR 5
	#define CC_ERROR 6
	#define CC_FATAL 7
	#define CC_REDISPLAY 8
	#define CC_REFRESH_BEEP 9

	// el_set/el_get parameters
	#define EL_PROMPT 0 // , el_pfunc_t
	#define EL_TERMINAL 1 // , const char *
	#define EL_EDITOR 2 // , const char *
	#define EL_SIGNAL 3 // , int);
	#define EL_BIND 4 // , const char *, ..., NULL
	#define EL_TELLTC 5 // , const char *, ..., NULL
	#define EL_SETTC 6 // , const char *, ..., NULL
	#define EL_ECHOTC 7 // , const char *, ..., NULL
	#define EL_SETTY 8 // , const char *, ..., NULL
	#define EL_ADDFN 9 // , const char , const char , el_func_t
	#define EL_HIST 10 // , hist_fun_t, const char *
	#define EL_EDITMODE 11 // , int
	#define EL_RPROMPT 12 // , el_pfunc_t
	#define EL_GETCFN 13 // , el_rfunc_t
	#define EL_CLIENTDATA 14 // , void *
	#define EL_UNBUFFERED 15 // , int
	#define EL_PREP_TERM 16 // , int
	#define EL_GETTC 17 // , const char *, ..., NULL
	#define EL_GETFP 18 // , int, FILE **
	#define EL_SETFP 19 // , int, FILE *
	#define EL_REFRESH 20 // , void
	#define EL_PROMPT_ESC 21 // , prompt_func, Char); set/get

	#define EL_BUILTIN_GETCFN (NULL)

	// history defines
	#define H_FUNC 0 // , UTSL
	#define H_SETSIZE 1 // , const int
	#define H_GETSIZE 2 // , void
	#define H_FIRST 3 // , void
	#define H_LAST 4 // , void
	#define H_PREV 5 // , void
	#define H_NEXT 6 // , void
	#define H_CURR 8 // , const int
	#define H_SET 7 // , int
	#define H_ADD 9 // , const char *
	#define H_ENTER 10 // , const char *
	#define H_APPEND 11 // , const char *
	#define H_END 12 // , void
	#define H_NEXT_STR 13 // , const char *
	#define H_PREV_STR 14 // , const char *
	#define H_NEXT_EVENT 15 // , const int
	#define H_PREV_EVENT 16 // , const int
	#define H_LOAD 17 // , const char *
	#define H_SAVE 18 // , const char *
	#define H_CLEAR 19 // , void
	#define H_SETUNIQUE 20 // , int
	#define H_GETUNIQUE 21 // , void
	#define H_DEL 22 // , int

	struct EditLine {};

	struct LineInfo {
	const char *buffer;
	const char *cursor;
	const char *lastchar;
	};

	struct History {};

	struct HistEvent {
	int num;
	const char *str;
	};

	extern "C" {
	// edit line API
	EditLine el_init(const char , FILE , FILE , FILE *);
	const char el_gets(EditLine , int *);
	int el_set(EditLine *, int, ...);

	void el_end(EditLine *);
	void el_reset(EditLine *);
	int el_getc(EditLine , char );
	void el_push(EditLine , const char );
	void el_beep(EditLine *);
	int el_parse(EditLine , int, const char *);
	int el_get(EditLine *, int, ...);
	int el_source(EditLine , const char );
	void el_resize(EditLine *);
	const LineInfo el_line(EditLine );
	int el_insertstr(EditLine , const char );
	void el_deletestr(EditLine *, int);

	// history API
	History *history_init(void);
	void history_end(History *);
	int history(History , HistEvent , int, ...);
	}

lldb/source/Host/CMakeLists.txt

	Show First 20 Lines • Show All 52 Lines • ▼ Show 20 Lines

	add_host_subdirectory(posix			add_host_subdirectory(posix
	posix/ConnectionFileDescriptorPosix.cpp			posix/ConnectionFileDescriptorPosix.cpp
	)			)

	if (CMAKE_SYSTEM_NAME MATCHES "Windows")			if (CMAKE_SYSTEM_NAME MATCHES "Windows")
	add_host_subdirectory(windows			add_host_subdirectory(windows
	windows/ConnectionGenericFileWindows.cpp			windows/ConnectionGenericFileWindows.cpp
	windows/EditLineWin.cpp
	windows/FileSystem.cpp			windows/FileSystem.cpp
	windows/Host.cpp			windows/Host.cpp
	windows/HostInfoWindows.cpp			windows/HostInfoWindows.cpp
	windows/HostProcessWindows.cpp			windows/HostProcessWindows.cpp
	windows/HostThreadWindows.cpp			windows/HostThreadWindows.cpp
	windows/LockFileWindows.cpp			windows/LockFileWindows.cpp
	windows/PipeWindows.cpp			windows/PipeWindows.cpp
	windows/ProcessLauncherWindows.cpp			windows/ProcessLauncherWindows.cpp
	▲ Show 20 Lines • Show All 110 Lines • Show Last 20 Lines

lldb/source/Host/windows/EditLineWin.cpp

This file was deleted.

	//===-- EditLineWin.cpp ---------------------------------------------------===//
	//
	// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
	// See https://llvm.org/LICENSE.txt for license information.
	// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
	//
	//===----------------------------------------------------------------------===//

	// this file is only relevant for Visual C++
	#if defined(_WIN32)

	#include "lldb/Host/windows/windows.h"

	#include "lldb/Host/windows/editlinewin.h"
	#include "llvm/Support/ErrorHandling.h"
	#include <assert.h>
	#include <vector>

	// edit line EL_ADDFN function pointer type
	typedef unsigned char (el_addfn_func)(EditLine e, int ch);
	typedef const char (el_prompt_func)(EditLine *);

	// edit line wrapper binding container
	struct el_binding {
	//
	const char *name;
	const char *help;
	// function pointer to callback routine
	el_addfn_func func;
	// ascii key this function is bound to
	const char *key;
	};

	// stored key bindings
	static std::vector<el_binding *> _bindings;

	// TODO: this should in fact be related to the exact edit line context we create
	static void *clientData = NULL;

	// store the current prompt string
	// default to what we expect to receive anyway
	static const char *_prompt = "(lldb) ";

	#if !defined(_WIP_INPUT_METHOD)

	static char el_get_s(char buffer, int chars) { return gets_s(buffer, chars); }
	#else

	static void con_output(char _in) {
	HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
	DWORD written = 0;
	// get the cursor position
	CONSOLE_SCREEN_BUFFER_INFO info;
	GetConsoleScreenBufferInfo(hout, &info);
	// output this char
	WriteConsoleOutputCharacterA(hout, &_in, 1, info.dwCursorPosition, &written);
	// advance cursor position
	info.dwCursorPosition.X++;
	SetConsoleCursorPosition(hout, info.dwCursorPosition);
	}

	static void con_backspace(void) {
	HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
	DWORD written = 0;
	// get cursor position
	CONSOLE_SCREEN_BUFFER_INFO info;
	GetConsoleScreenBufferInfo(hout, &info);
	// nudge cursor backwards
	info.dwCursorPosition.X--;
	SetConsoleCursorPosition(hout, info.dwCursorPosition);
	// blank out the last character
	WriteConsoleOutputCharacterA(hout, " ", 1, info.dwCursorPosition, &written);
	}

	static void con_return(void) {
	HANDLE hout = GetStdHandle(STD_OUTPUT_HANDLE);
	DWORD written = 0;
	// get cursor position
	CONSOLE_SCREEN_BUFFER_INFO info;
	GetConsoleScreenBufferInfo(hout, &info);
	// move onto the new line
	info.dwCursorPosition.X = 0;
	info.dwCursorPosition.Y++;
	SetConsoleCursorPosition(hout, info.dwCursorPosition);
	}

	static bool runBind(char _key) {
	for (int i = 0; i < _bindings.size(); i++) {
	el_binding *bind = _bindings[i];
	if (bind->key[0] == _key) {
	bind->func((EditLine *)-1, _key);
	return true;
	}
	}
	return false;
	}

	// replacement get_s which is EL_BIND aware
	static char el_get_s(char buffer, int chars) {
	//
	char *head = buffer;
	//
	for (;; Sleep(10)) {
	//
	INPUT_RECORD _record;
	//
	DWORD _read = 0;
	if (ReadConsoleInputA(GetStdHandle(STD_INPUT_HANDLE), &_record, 1,
	&_read) == FALSE)
	break;
	// if we didn't read a key
	if (_read == 0)
	continue;
	// only interested in key events
	if (_record.EventType != KEY_EVENT)
	continue;
	// is the key down
	if (!_record.Event.KeyEvent.bKeyDown)
	continue;
	// read the ascii key character
	char _key = _record.Event.KeyEvent.uChar.AsciiChar;
	// non ascii conformant key press
	if (_key == 0) {
	// check the scan code
	// if VK_UP scroll back through history
	// if VK_DOWN scroll forward through history
	continue;
	}
	// try to execute any bind this key may have
	if (runBind(_key))
	continue;
	// if we read a return key
	if (_key == '\n' \|\| _key == '\r') {
	con_return();
	break;
	}
	// key is backspace
	if (_key == 0x8) {
	// avoid deleting past beginning
	if (head > buffer) {
	con_backspace();
	head--;
	}
	continue;
	}

	// add this key to the input buffer
	if ((head - buffer) < (chars - 1)) {
	con_output(_key);
	*(head++) = _key;
	}
	}
	// insert end of line character
	*head = '\0';

	return buffer;
	}
	#endif

	// edit line initialize
	EditLine el_init(const char , FILE , FILE , FILE *) {
	//
	SetConsoleTitleA("lldb");
	// return dummy handle
	return (EditLine *)-1;
	}

	const char el_gets(EditLine el, int *length) {
	// print the prompt if we have one
	if (_prompt != NULL)
	printf("%s", _prompt);
	// create a buffer for the user input
	char *buffer = new char[MAX_PATH];
	// try to get user input string
	if (el_get_s(buffer, MAX_PATH)) {
	// get the string length in 'length'
	while (buffer[*length] != '\0')
	(*length)++;
	// return the input buffer
	// remember that this memory has the be free'd somewhere
	return buffer;
	} else {
	// on error
	delete[] buffer;
	return NULL;
	}
	}

	int el_set(EditLine *el, int code, ...) {
	va_list vl;
	va_start(vl, code);
	//
	switch (code) {
	// edit line set prompt message
	case (EL_PROMPT): {
	// EL_PROMPT, char (f)( EditLine *)
	// define a prompt printing function as 'f', which is to return a
	// string that
	// contains the prompt.

	// get the function pointer from the arg list
	void func_vp = (void )va_arg(vl, el_prompt_func);
	// cast to suitable prototype
	el_prompt_func func_fp = (el_prompt_func)func_vp;
	// call to get the prompt as a string
	_prompt = func_fp(el);
	} break;

	case (EL_PROMPT_ESC): {
	// EL_PROMPT, char (f)( EditLine *)
	// define a prompt printing function as 'f', which is to return a
	// string that
	// contains the prompt.

	// get the function pointer from the arg list
	void func_vp = (void )va_arg(vl, el_prompt_func);
	va_arg(vl, int);
	// call to get the prompt as a string
	el_prompt_func func_fp = (el_prompt_func)func_vp;
	_prompt = func_fp(el);
	} break;

	case (EL_EDITOR): {
	// EL_EDITOR, const char *mode
	// set editing mode to "emacs" or "vi"
	} break;
	case (EL_HIST): {
	// EL_HIST, History (fun)(History , int op, ... ), const char ptr
	// defines which history function to use, which is usually history().
	// Ptr should be the
	// value returned by history_init().
	} break;
	case (EL_ADDFN): {
	// EL_ADDFN, const char name, const char help, unsigned char
	// (func)(EditLine e, int ch)
	// add a user defined function, func), referred to as 'name' which is
	// invoked when a key which is bound to 'name' is
	// entered. 'help' is a description of 'name'. at invocation time, 'ch'
	// is the key which caused the invocation. the
	// return value of 'func()' should be one of:
	// CC_NORM add a normal character
	// CC_NEWLINE end of line was entered
	// CC_EOF EOF was entered
	// CC_ARGHACK expecting further command input as arguments, do
	// nothing visually.
	// CC_REFRESH refresh display.
	// CC_REFRESH_BEEP refresh display and beep.
	// CC_CURSOR cursor moved so update and perform CC_REFRESH
	// CC_REDISPLAY redisplay entire input line. this is useful
	// if a key binding outputs extra information.
	// CC_ERROR an error occurred. beep and flush tty.
	// CC_FATAL fatal error, reset tty to known state.

	el_binding *binding = new el_binding;
	binding->name = va_arg(vl, const char *);
	binding->help = va_arg(vl, const char *);
	binding->func = va_arg(vl, el_addfn_func);
	binding->key = 0;
	// add this to the bindings list
	_bindings.push_back(binding);
	} break;
	case (EL_BIND): {
	// EL_BIND, const char *, ..., NULL
	// perform the BIND built-in command. Refer to editrc(5) for more
	// information.

	const char name = va_arg(vl, const char );

	for (auto bind : _bindings) {
	if (strcmp(bind->name, name) == 0) {
	bind->key = va_arg(vl, const char *);
	break;
	}
	}

	} break;
	case (EL_CLIENTDATA): {
	clientData = va_arg(vl, void *);
	} break;
	}
	return 0;
	}

	void el_end(EditLine *el) {
	// assert( !"Not implemented!" );
	}

	void el_reset(EditLine *) { llvm_unreachable("Not implemented!"); }

	int el_getc(EditLine , char ) {
	llvm_unreachable("Not implemented!");
	}

	void el_push(EditLine , const char ) {}

	void el_beep(EditLine *) { Beep(1000, 500); }

	int el_parse(EditLine , int, const char *) {
	llvm_unreachable("Not implemented!");
	}

	int el_get(EditLine *el, int code, ...) {
	va_list vl;
	va_start(vl, code);

	switch (code) {
	case (EL_CLIENTDATA): {
	void dout = va_arg(vl, void );
	*dout = clientData;
	} break;
	default:
	llvm_unreachable("Not implemented!");
	}
	return 0;
	}

	int el_source(EditLine el, const char file) {
	// init edit line by reading the contents of 'file' nothing to do here on
	// windows...
	return 0;
	}

	void el_resize(EditLine *) { llvm_unreachable("Not implemented!"); }

	const LineInfo el_line(EditLine el) { return 0; }

	int el_insertstr(EditLine , const char ) {
	// assert( !"Not implemented!" );
	return 0;
	}

	void el_deletestr(EditLine *, int) { llvm_unreachable("Not implemented!"); }

	History *history_init(void) {
	// return dummy handle
	return (History *)-1;
	}

	void history_end(History *) {
	// assert( !"Not implemented!" );
	}

	int history(History , HistEvent , int op, ...) {
	// perform operation 'op' on the history list with optional arguments as
	// needed by the operation.
	return 0;
	}

	#endif