Commit e1a60bbe authored by da-woods's avatar da-woods Committed by GitHub

Resolve some issues with "cpp_locals" (GH-4265)

* Fix class attributes access where cpp_locals=False.
* Add "no-cpp-locals" tag to mark a test as not suitable for running with "cpp_locals=True".
* Add a list of "extra_directives" to runtests as additional test mode.

Resolves some issues in https://github.com/cython/cython/issues/4266
parent 9f78ed07
......@@ -849,7 +849,7 @@ class FunctionState(object):
elif type.is_cfunction:
from . import PyrexTypes
type = PyrexTypes.c_ptr_type(type) # A function itself isn't an l-value
elif type.is_cpp_class and self.scope.directives['cpp_locals']:
elif type.is_cpp_class and not type.is_fake_reference and self.scope.directives['cpp_locals']:
self.scope.use_utility_code(UtilityCode.load_cached("OptionalLocals", "CppSupport.cpp"))
if not type.is_pyobject and not type.is_memoryviewslice:
# Make manage_ref canonical, so that manage_ref will always mean
......@@ -2087,7 +2087,7 @@ class CCodeWriter(object):
def put_temp_declarations(self, func_context):
for name, type, manage_ref, static in func_context.temps_allocated:
if type.is_cpp_class and func_context.scope.directives['cpp_locals']:
if type.is_cpp_class and not type.is_fake_reference and func_context.scope.directives['cpp_locals']:
decl = type.cpp_optional_declaration_code(name)
else:
decl = type.declaration_code(name)
......
......@@ -2288,6 +2288,8 @@ class NameNode(AtomicExprNode):
entry = self.entry
if not entry:
return "<error>" # There was an error earlier
if self.entry.is_cpp_optional and not self.is_target:
return "(*%s)" % entry.cname
return entry.cname
def generate_result_code(self, code):
......@@ -2897,6 +2899,7 @@ class CppIteratorNode(ExprNode):
# Created at the analyse_types stage by IteratorNode
cpp_sequence_cname = None
cpp_attribute_op = "."
extra_dereference = ""
is_temp = True
subexprs = ['sequence']
......@@ -2921,6 +2924,8 @@ class CppIteratorNode(ExprNode):
return self
iter_type = begin.type.return_type
if iter_type.is_cpp_class:
if env.directives['cpp_locals']:
self.extra_dereference = "*"
if env.lookup_operator_for_types(
self.pos,
"!=",
......@@ -2965,7 +2970,7 @@ class CppIteratorNode(ExprNode):
# make the temp a pointer so we are not sensitive to users reassigning
# the pointer than it came from
temp_type = PyrexTypes.CPtrType(sequence_type.ref_base_type)
if temp_type.is_ptr:
if temp_type.is_ptr or code.globalstate.directives['cpp_locals']:
self.cpp_attribute_op = "->"
# 3) (otherwise) sequence comes from a function call or similar, so we must
# create a temp to store it in
......@@ -2979,14 +2984,16 @@ class CppIteratorNode(ExprNode):
def generate_iter_next_result_code(self, result_name, code):
# end call isn't cached to support containers that allow adding while iterating
# (much as this is usually a bad idea)
code.putln("if (!(%s != %s%send())) break;" % (
code.putln("if (!(%s%s != %s%send())) break;" % (
self.extra_dereference,
self.result(),
self.cpp_sequence_cname or self.sequence.result(),
self.cpp_attribute_op))
code.putln("%s = *%s;" % (
code.putln("%s = *%s%s;" % (
result_name,
self.extra_dereference,
self.result()))
code.putln("++%s;" % self.result())
code.putln("++%s%s;" % (self.extra_dereference, self.result()))
def free_temps(self, code):
if self.cpp_sequence_cname:
......@@ -7162,9 +7169,6 @@ class AttributeNode(ExprNode):
self.op = "->"
elif obj_type.is_reference and obj_type.is_fake_reference:
self.op = "->"
elif (obj_type.is_cpp_class and (self.obj.is_name or self.obj.is_attribute) and
self.obj.entry and self.obj.entry.is_cpp_optional):
self.op = "->"
else:
self.op = "."
if obj_type.has_attributes:
......@@ -7297,9 +7301,14 @@ class AttributeNode(ExprNode):
return NameNode.is_ephemeral(self)
def calculate_result_code(self):
#print "AttributeNode.calculate_result_code:", self.member ###
#print "...obj node =", self.obj, "code", self.obj.result() ###
#print "...obj type", self.obj.type, "ctype", self.obj.ctype() ###
result = self.calculate_access_code()
if self.entry and self.entry.is_cpp_optional and not self.is_target:
result = "(*%s)" % result
return result
def calculate_access_code(self):
# Does the job of calculate_result_code but doesn't dereference cpp_optionals
# Therefore allowing access to the holder variable
obj = self.obj
obj_code = obj.result_as(obj.type)
#print "...obj_code =", obj_code ###
......@@ -7374,7 +7383,12 @@ class AttributeNode(ExprNode):
'%s'
'}' % (self.result(), code.error_goto(self.pos)))
elif self.entry.is_cpp_optional and self.initialized_check:
unbound_check_code = self.type.cpp_optional_check_for_null_code(self.result())
if self.is_target:
undereferenced_result = self.result()
else:
assert not self.is_temp # calculate_access_code() only makes sense for non-temps
undereferenced_result = self.calculate_access_code()
unbound_check_code = self.type.cpp_optional_check_for_null_code(undereferenced_result)
code.put_error_if_unbound(self.pos, self.entry, unbound_check_code=unbound_check_code)
else:
# result_code contains what is needed, but we may need to insert
......
......@@ -3166,7 +3166,9 @@ class CoerceCppTemps(EnvTransform, SkipDeclarations):
def visit_ExprNode(self, node):
self.visitchildren(node)
if (self.current_env().directives['cpp_locals'] and
node.is_temp and node.type.is_cpp_class):
node.is_temp and node.type.is_cpp_class and
# Fake references are not replaced with "std::optional()".
not node.type.is_fake_reference):
node = ExprNodes.CppOptionalTempCoercion(node)
return node
......
......@@ -248,6 +248,7 @@ class PyrexType(BaseType):
is_ptr = 0
is_null_ptr = 0
is_reference = 0
is_fake_reference = 0
is_rvalue_reference = 0
is_const = 0
is_volatile = 0
......
......@@ -300,6 +300,7 @@ class InnerEntry(Entry):
self.cf_assignments = outermost_entry.cf_assignments
self.cf_references = outermost_entry.cf_references
self.overloaded_alternatives = outermost_entry.overloaded_alternatives
self.is_cpp_optional = outermost_entry.is_cpp_optional
self.inner_entries.append(self)
def __getattr__(self, name):
......@@ -1925,6 +1926,8 @@ class LocalScope(Scope):
elif entry.in_closure:
entry.original_cname = entry.cname
entry.cname = "%s->%s" % (Naming.cur_scope_cname, entry.cname)
if entry.type.is_cpp_class and entry.scope.directives['cpp_locals']:
entry.make_cpp_optional()
class ComprehensionScope(Scope):
......
......@@ -297,18 +297,20 @@ def update_cpp11_extension(ext):
update cpp11 extensions that will run on versions of gcc >4.8
"""
gcc_version = get_gcc_version(ext.language)
already_has_std = any(ca for ca in ext.extra_compile_args if "-std" in ca)
if gcc_version:
compiler_version = gcc_version.group(1)
if float(compiler_version) > 4.8:
if float(compiler_version) > 4.8 and not already_has_std:
ext.extra_compile_args.append("-std=c++11")
return ext
clang_version = get_clang_version(ext.language)
if clang_version:
ext.extra_compile_args.append("-std=c++11")
if not already_has_std:
ext.extra_compile_args.append("-std=c++11")
if sys.platform == "darwin":
ext.extra_compile_args.append("-stdlib=libc++")
ext.extra_compile_args.append("-mmacosx-version-min=10.7")
ext.extra_compile_args.append("-stdlib=libc++")
ext.extra_compile_args.append("-mmacosx-version-min=10.7")
return ext
return EXCLUDE_EXT
......@@ -320,6 +322,10 @@ def update_cpp17_extension(ext):
gcc_version = get_gcc_version(ext.language)
if gcc_version:
compiler_version = gcc_version.group(1)
if sys.version_info[0] < 3:
# The Python 2.7 headers contain the 'register' modifier
# which gcc warns about in C++17 mode.
ext.extra_compile_args.append('-Wno-register')
if float(compiler_version) >= 5.0:
ext.extra_compile_args.append("-std=c++17")
return ext
......@@ -678,7 +684,8 @@ class TestBuilder(object):
with_pyregr, languages, test_bugs, language_level,
common_utility_dir, pythran_dir=None,
default_mode='run', stats=None,
add_embedded_test=False, add_cython_import=False):
add_embedded_test=False, add_cython_import=False,
add_cpp_locals_extra_tests=False):
self.rootdir = rootdir
self.workdir = workdir
self.selectors = selectors
......@@ -702,6 +709,7 @@ class TestBuilder(object):
self.add_embedded_test = add_embedded_test
self.add_cython_import = add_cython_import
self.capture = options.capture
self.add_cpp_locals_extra_tests = add_cpp_locals_extra_tests
def build_suite(self):
suite = unittest.TestSuite()
......@@ -805,6 +813,8 @@ class TestBuilder(object):
warning_errors = 'werror' in tags['tag']
expect_warnings = 'warnings' in tags['tag']
extra_directives_list = [{}]
if expect_errors:
if skip_c(tags) and 'cpp' in self.languages:
languages = ['cpp']
......@@ -819,6 +829,9 @@ class TestBuilder(object):
if 'cpp' in languages and 'no-cpp' in tags['tag']:
languages = list(languages)
languages.remove('cpp')
if (self.add_cpp_locals_extra_tests and 'cpp' in languages and
'cpp' in tags['tag'] and not 'no-cpp-locals' in tags['tag']):
extra_directives_list.append({'cpp_locals': True})
if not languages:
return []
......@@ -840,15 +853,18 @@ class TestBuilder(object):
tags, language, language_level,
expect_errors, expect_warnings, warning_errors, preparse,
pythran_dir if language == "cpp" else None,
add_cython_import=add_cython_import)
add_cython_import=add_cython_import,
extra_directives=extra_directives)
for language in languages
for preparse in preparse_list
for language_level in language_levels
for extra_directives in extra_directives_list
]
return tests
def build_test(self, test_class, path, workdir, module, module_path, tags, language, language_level,
expect_errors, expect_warnings, warning_errors, preparse, pythran_dir, add_cython_import):
expect_errors, expect_warnings, warning_errors, preparse, pythran_dir, add_cython_import,
extra_directives):
language_workdir = os.path.join(workdir, language)
if not os.path.exists(language_workdir):
os.makedirs(language_workdir)
......@@ -857,6 +873,8 @@ class TestBuilder(object):
workdir += '_%s' % (preparse,)
if language_level:
workdir += '_cy%d' % (language_level,)
if extra_directives:
workdir += ('_directives_'+ '_'.join('%s_%s' % (k, v) for k,v in extra_directives.items()))
return test_class(path, workdir, module, module_path, tags,
language=language,
preparse=preparse,
......@@ -924,7 +942,8 @@ class CythonCompileTestCase(unittest.TestCase):
cleanup_sharedlibs=True, cleanup_failures=True, cython_only=False, test_selector=None,
fork=True, language_level=2, warning_errors=False,
test_determinism=False,
common_utility_dir=None, pythran_dir=None, stats=None, add_cython_import=False):
common_utility_dir=None, pythran_dir=None, stats=None, add_cython_import=False,
extra_directives={}):
self.test_directory = test_directory
self.tags = tags
self.workdir = workdir
......@@ -949,6 +968,7 @@ class CythonCompileTestCase(unittest.TestCase):
self.pythran_dir = pythran_dir
self.stats = stats
self.add_cython_import = add_cython_import
self.extra_directives = extra_directives
unittest.TestCase.__init__(self)
def shortDescription(self):
......@@ -978,6 +998,7 @@ class CythonCompileTestCase(unittest.TestCase):
Options.warning_errors = self.warning_errors
if sys.version_info >= (3, 4):
Options._directive_defaults['autotestdict'] = False
Options._directive_defaults.update(self.extra_directives)
if not os.path.exists(self.workdir):
os.makedirs(self.workdir)
......@@ -1219,6 +1240,10 @@ class CythonCompileTestCase(unittest.TestCase):
if self.language == 'cpp':
# Set the language now as the fixer might need it
extension.language = 'c++'
if self.extra_directives.get('cpp_locals'):
extension = update_cpp17_extension(extension)
if extension is EXCLUDE_EXT:
return
if 'distutils' in self.tags:
from Cython.Build.Dependencies import DistutilsInfo
......@@ -2193,6 +2218,9 @@ def main():
parser.add_option("--no-cpp", dest="use_cpp",
action="store_false", default=True,
help="do not test C++ compilation backend")
parser.add_option("--no-cpp-locals", dest="use_cpp_locals",
action="store_false", default=True,
help="do not rerun select C++ tests with cpp_locals directive")
parser.add_option("--no-unit", dest="unittests",
action="store_false", default=True,
help="do not run the unit tests")
......@@ -2670,7 +2698,8 @@ def runtests(options, cmd_args, coverage=None):
filetests = TestBuilder(ROOTDIR, WORKDIR, selectors, exclude_selectors,
options, options.pyregr, languages, test_bugs,
options.language_level, common_utility_dir,
options.pythran_dir, add_embedded_test=True, stats=stats)
options.pythran_dir, add_embedded_test=True, stats=stats,
add_cpp_locals_extra_tests=options.use_cpp_locals)
test_suite.addTest(filetests.build_suite())
if options.examples and languages:
......
# tag: cpp,cpp11
# mode: compile
# mode: compile
# tag: no-cpp-locals
# TODO cpp_locals works fine with the standard library that comes with gcc11
# but not with gcc8. Therefore disable the test for now
cdef extern from *:
"""
......
# mode: error
# tag: cpp, werror, numpy
# tag: cpp, werror, numpy, no-cpp-locals
import numpy as np
cimport numpy as np
......
# cython: warn.maybe_uninitialized=True
# mode: error
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
# FIXME - no-cpp-locals should work
from cython.operator import typeid
......
# mode: run
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
from libcpp cimport bool
......
# mode: run
# tag: cpp, warnings
# tag: cpp, warnings, no-cpp-locals
# This gives a warning about the previous .pxd definition, but should not give an error.
cdef cppclass Foo:
......
# mode: run
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
from libcpp.vector cimport vector
......
# mode: run
# tag: cpp, werror, cpp11
# tag: cpp, werror, cpp11, no-cpp-locals
# cython: experimental_cpp_class_def=True
cdef double pi
......
# mode: run
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
# cython: experimental_cpp_class_def=True
from libcpp.vector cimport vector
......@@ -82,6 +82,6 @@ def test_vector_members(py_a, py_b):
cdef vector_members(vector[const Wrapper[int]*] a, const vector[wrapInt*] b):
# TODO: Cython-level error.
# b[0].set(100)
# TODO: const_iterator
return [x.get() for x in a], b[0].get()
# tag: cpp
# mode: run
# mode: run, no-cpp-locals
cdef extern from *:
"""
......
# mode: run
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
# ticket: 3065
# This is intentionally in a file on its own. The issue was that it failed to generate utility-code
......
# mode: run
# tag: cpp, cpp11
# tag: cpp, cpp11, no-cpp-locals
from libcpp.utility cimport move
......
# mode: run
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
from libcpp.deque cimport deque
from libcpp.vector cimport vector
......
# mode: run
# tag: cpp, cpp17
# tag: cpp, cpp17, no-cpp-locals
# no-cpp-locals because the test is already run with it explicitly set
# cython: cpp_locals=True
......@@ -48,7 +49,7 @@ cdef extern from *:
C make_C(int) except + # needs a temp to receive
# this function just makes sure the output from the destructor can be captured by doctest
cdef void print_C_destructor "print_C_destructor" () nogil:
cdef void print_C_destructor "print_C_destructor" () with gil:
print("~C()")
def maybe_assign_infer(assign, value, do_print):
......@@ -167,11 +168,20 @@ cdef class HoldsC:
>>> inst = HoldsC(True, False)
>>> inst.getCX()
10
>>> access_from_function_with_different_directive(inst)
10
10
>>> inst.getCX() # it was changed in access_from_function_with_different_directive
20
>>> inst = HoldsC(False, False)
>>> inst.getCX()
Traceback (most recent call last):
...
AttributeError: C++ attribute 'value' is not initialized
>>> access_from_function_with_different_directive(inst)
Traceback (most recent call last):
...
AttributeError: C++ attribute 'value' is not initialized
"""
cdef C value
def __cinit__(self, initialize, print_destructor):
......@@ -181,6 +191,16 @@ cdef class HoldsC:
def getCX(self):
return self.value.getX()
cdef acceptC(C& c):
return c.getX()
@cython.cpp_locals(False)
def access_from_function_with_different_directive(HoldsC c):
# doctest is in HoldsC class
print(acceptC(c.value)) # this originally tried to pass a __Pyx_Optional<C> as a C instance
print(c.value.getX())
c.value = C(20, False) # make sure that we can change it too
def dont_test_on_pypy(f):
import sys
if not hasattr(sys, "pypy_version_info"):
......
# mode: run
# tag: cpp, cpp17
# tag: cpp, cpp17, no-cpp-locals
# no cpp_locals because this test is already run with cpp_locals explicitly set
# cython: cpp_locals=True
......
# mode: run
# tag: cpp, werror, cpp11
# tag: cpp, werror, cpp11, no-cpp-locals
from libcpp cimport nullptr
from libcpp.memory cimport shared_ptr, make_shared
......
# tag: cpp
# tag: cpp, no-cpp-locals
cdef extern from "cpp_nested_classes_support.h":
cdef cppclass A:
......
# tag: cpp
# tag: cpp, no-cpp-locals
cdef extern from "cpp_nonstdint.h":
ctypedef int Int24
......
# mode: run
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
from cython.operator import (preincrement, predecrement,
postincrement, postdecrement)
......
# mode: run
# tag: cpp, werror
# tag: no-cpp-locals
# FIXME - cpp_locals should work but doesn't
from __future__ import division
......
# mode: run
# tag: cpp, werror, cpp11
# tag: cpp, werror, cpp11, no-cpp-locals
from libcpp.memory cimport unique_ptr, shared_ptr, default_delete, dynamic_pointer_cast
from libcpp cimport nullptr
......
# mode: run
# tag: cpp
# tag: cpp, no-cpp-locals
cdef extern from *:
"""
......
# mode: run
# tag: cpp, werror, cpp17, cppexecpolicies
# tag: cpp, werror, cpp17, cppexecpolicies, no-cpp-locals
from libcpp.algorithm cimport is_sorted, sort, stable_sort, nth_element, all_of, count, copy
from libcpp.execution cimport seq
......
# mode: run
# tag: cpp, werror, cpp11
# tag: cpp, werror, cpp11, no-cpp-locals
from __future__ import print_function
......
# mode: run
# tag: cpp, werror, cpp11
# tag: cpp, werror, cpp11, no-cpp-locals
from __future__ import print_function
......
# mode: run
# tag: cpp, werror, cpp11
# tag: cpp, werror, cpp11, no-cpp-locals
from __future__ import print_function
......
# mode: run
# tag: cpp, cpp11, werror
# tag: cpp, cpp11, werror, no-cpp-locals
from cython.operator cimport preincrement as incr, dereference as deref
from libc.stdint cimport *
......
# mode: run
# tag: cpp, werror, cpp11
# tag: cpp, werror, cpp11, no-cpp-locals
import sys
from libcpp.unordered_map cimport unordered_map
......
# mode: run
# tag: cpp, werror, cpp11
# tag: cpp, werror, cpp11, no-cpp-locals
from cython.operator cimport dereference as deref
from cython.operator cimport preincrement as incr
......
# mode: run
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
from cython.operator cimport dereference as deref
from cython.operator cimport preincrement as incr
......
# mode: run
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
from cython.operator cimport dereference as d
from cython.operator cimport preincrement as incr
......
# mode: run
# tag: cpp, warnings
# tag: cpp, warnings, no-cpp-locals
cimport cython
from libcpp.pair cimport pair
......
# tag: cpp
# tag: cpp, no-cpp-locals
from libcpp.vector cimport vector
......
# mode: run
# tag: cpp, werror
# tag: cpp, werror, no-cpp-locals
from cython.operator import dereference as deref
from libcpp.pair cimport pair
......
# tag: cpp
# tag: cpp, no-cpp-locals
# FIXME (or at least investigate) - cpp_locals should probably work
cimport cython
from libcpp.vector cimport vector
......
# tag: cpp
# tag: cpp, no-cpp-locals
import cython
......
# tag: cpp
# tag: cpp, no-cpp-locals
from libcpp.vector cimport vector
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment