Skip to content

Commit

Permalink
[lldb] Improve error message for modules with dots or dashes
Browse files Browse the repository at this point in the history
LLDB does not like to import Python files with dashes or dots in their
name. While the former are technically allowed, it is discouraged. Dots
are allowed for subpackages but not in module names. This patch improves
the user experience by printing a useful error.

Before this patch:

  error: module importing failed: SyntaxError('invalid syntax',
  ('<string>', 1, 11, 'import foo-bar\n'))

After this patch:

  error: module importing failed: Python discourages dashes in module
  names: foo-bar

rdar://74263511

[1] https://www.python.org/dev/peps/pep-0008/#package-and-module-names

Differential revision: https://reviews.llvm.org/D96833
  • Loading branch information
JDevlieghere committed Feb 17, 2021
1 parent 8a783e6 commit d6e8057
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2781,6 +2781,7 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule(
};

std::string module_name(pathname);
bool possible_package = false;

if (extra_search_dir) {
if (llvm::Error e = ExtendSysPath(extra_search_dir.GetPath())) {
Expand All @@ -2805,6 +2806,7 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule(
return false;
}
// Not a filename, probably a package of some sort, let it go through.
possible_package = true;
} else if (is_directory(st) || is_regular_file(st)) {
if (module_file.GetDirectory().IsEmpty()) {
error.SetErrorString("invalid directory name");
Expand All @@ -2831,6 +2833,18 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule(
module_name.resize(module_name.length() - 4);
}

if (!possible_package && module_name.find('.') != llvm::StringRef::npos) {
error.SetErrorStringWithFormat(
"Python does not allow dots in module names: %s", module_name.c_str());
return false;
}

if (module_name.find('-') != llvm::StringRef::npos) {
error.SetErrorStringWithFormat(
"Python discourages dashes in module names: %s", module_name.c_str());
return false;
}

// check if the module is already import-ed
StreamString command_stream;
command_stream.Clear();
Expand Down
20 changes: 20 additions & 0 deletions lldb/test/Shell/ScriptInterpreter/Python/command_import.test
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# REQUIRES: python

# RUN: rm -rf %t && mkdir -p %t
# RUN: echo "print('Rene Magritte')" >> %t/foo.py
# RUN: echo "print('Jan van Eyck')" >> %t/foo-bar.py
# RUN: echo "print('Pieter Bruegel the Elder')" >> %t/foo.bar.py
# RUN: echo "print('Pieter Bruegel the Younger')" >> %t/foo.bar

# RUN: %lldb --script-language python -o 'command script import %t/foo.py' 2>&1 | FileCheck %s --check-prefix MAGRITTE
# MAGRITTE: Rene Magritte

# RUN: %lldb --script-language python -o 'command script import %t/foo-bar.py' 2>&1 | FileCheck %s --check-prefix VANEYCK
# VANEYCK-NOT: Jan van Eyck
# VANEYCK: module importing failed: Python discourages dashes in module names: foo-bar

# RUN: %lldb --script-language python -o 'command script import %t/foo.bar.py' 2>&1 | FileCheck %s --check-prefix BRUEGEL
# RUN: %lldb --script-language python -o 'command script import %t/foo.bar' 2>&1 | FileCheck %s --check-prefix BRUEGEL
# BRUEGEL-NOT: Pieter Bruegel the Elder
# BRUEGEL-NOT: Pieter Bruegel the Younger
# BRUEGEL: module importing failed: Python does not allow dots in module names: foo.bar

0 comments on commit d6e8057

Please sign in to comment.