Skip to content

Commit

Permalink
THRIFT-5715 Python flag to generate mutable exceptions
Browse files Browse the repository at this point in the history
In Python 3.11 exceptions generated by the compiler can't be used with a
context manager because they are immutable. As of Python 3.11
`contextlib.contextmanager` sets `exc.__stacktrace__` in the event that
the code in the context manager errors.

As of Thrift v0.18.1 exceptions are generated as immutable by default.
See [PR#1835](#1835) for more
information about why exceptions were made immutable by default.

This change adds a compiler flag, `py:immutible_exc` that generates all
exceptions as mutable so they can be used with a context manager. When
the flag is used with the `python.immutable = "true"` annotation, the
exception is treated as immutable.

I tested this change by generating the tutorial and comparing the
outputs with and without the new flag. I also copied
[tutorial.thrift](tutorial/tutorial.thrift) and added the
`python.immutable` annotation to ensure that the annotation always
overrides the flag.

Where do I go to update documentation?
  • Loading branch information
KTAtkinson committed Jun 14, 2023
1 parent dd53b94 commit 402f505
Showing 1 changed file with 34 additions and 26 deletions.
60 changes: 34 additions & 26 deletions compiler/cpp/src/thrift/generate/t_py_generator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ class t_py_generator : public t_generator {
gen_twisted_ = false;
gen_dynamic_ = false;
gen_enum_ = false;
gen_immutable_exc_ = true;
coding_ = "";
gen_dynbaseclass_ = "";
gen_dynbaseclass_exc_ = "";
Expand Down Expand Up @@ -105,7 +106,9 @@ class t_py_generator : public t_generator {
}
if( import_dynbase_.empty()) {
import_dynbase_ = "from thrift.protocol.TBase import TBase, TFrozenBase, TExceptionBase, TFrozenExceptionBase, TTransport\n";
}
}
} else if( iter->first.compare("mutable_exc") == 0) {
gen_immutable_exc_ = false;
} else if( iter->first.compare("dynbase") == 0) {
gen_dynbase_ = true;
gen_dynbaseclass_ = (iter->second);
Expand Down Expand Up @@ -280,12 +283,12 @@ class t_py_generator : public t_generator {
return package_dir + real_module;
}

static bool is_immutable(t_type* ttype) {
static bool is_immutable(t_type* ttype, bool immutable_exc) {
std::map<std::string, std::vector<std::string>>::iterator it = ttype->annotations_.find("python.immutable");

if (it == ttype->annotations_.end()) {
// Exceptions are immutable by default.
return ttype->is_xception();
// Exceptions are immutable by default
return immutable_exc && ttype->is_xception();
} else if (!it->second.empty() && it->second.back() == "false") {
return false;
} else {
Expand Down Expand Up @@ -314,6 +317,11 @@ class t_py_generator : public t_generator {

std::string import_dynbase_;

/**
* False if exceptions should be mutable, defaults to true.
*/
bool gen_immutable_exc_;

bool gen_slots_;

std::string copy_options_;
Expand Down Expand Up @@ -636,7 +644,7 @@ string t_py_generator::render_const_value(t_type* type, t_const_value* value) {
} else if (type->is_map()) {
t_type* ktype = ((t_map*)type)->get_key_type();
t_type* vtype = ((t_map*)type)->get_val_type();
if (is_immutable(type)) {
if (is_immutable(type, gen_immutable_exc_)) {
out << "TFrozenDict(";
}
out << "{" << endl;
Expand All @@ -649,7 +657,7 @@ string t_py_generator::render_const_value(t_type* type, t_const_value* value) {
}
indent_down();
indent(out) << "}";
if (is_immutable(type)) {
if (is_immutable(type, gen_immutable_exc_)) {
out << ")";
}
} else if (type->is_list() || type->is_set()) {
Expand All @@ -660,12 +668,12 @@ string t_py_generator::render_const_value(t_type* type, t_const_value* value) {
etype = ((t_set*)type)->get_elem_type();
}
if (type->is_set()) {
if (is_immutable(type)) {
if (is_immutable(type, gen_immutable_exc_)) {
out << "frozen";
}
out << "set(";
}
if (is_immutable(type) || type->is_set()) {
if (is_immutable(type, gen_immutable_exc_) || type->is_set()) {
out << "(" << endl;
} else {
out << "[" << endl;
Expand All @@ -677,7 +685,7 @@ string t_py_generator::render_const_value(t_type* type, t_const_value* value) {
indent(out) << render_const_value(etype, *v_iter) << "," << endl;
}
indent_down();
if (is_immutable(type) || type->is_set()) {
if (is_immutable(type, gen_immutable_exc_) || type->is_set()) {
indent(out) << ")";
} else {
indent(out) << "]";
Expand Down Expand Up @@ -789,7 +797,7 @@ void t_py_generator::generate_py_struct_definition(ostream& out,
out << endl << endl << "class " << tstruct->get_name();
if (is_exception) {
if (gen_dynamic_) {
if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
out << "(" << gen_dynbaseclass_frozen_exc_ << ")";
} else {
out << "(" << gen_dynbaseclass_exc_ << ")";
Expand All @@ -798,7 +806,7 @@ void t_py_generator::generate_py_struct_definition(ostream& out,
out << "(TException)";
}
} else if (gen_dynamic_) {
if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
out << "(" << gen_dynbaseclass_frozen_ << ")";
} else {
out << "(" << gen_dynbaseclass_ << ")";
Expand Down Expand Up @@ -870,7 +878,7 @@ void t_py_generator::generate_py_struct_definition(ostream& out,
indent_down();
}

if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
if (gen_newstyle_ || gen_dynamic_) {
indent(out) << "super(" << tstruct->get_name() << ", self).__setattr__('"
<< (*m_iter)->get_name() << "', " << (*m_iter)->get_name() << ")" << endl;
Expand All @@ -886,7 +894,7 @@ void t_py_generator::generate_py_struct_definition(ostream& out,
indent_down();
}

if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
out << endl;
out << indent() << "def __setattr__(self, *args):" << endl
<< indent() << indent_str() << "raise TypeError(\"can't modify immutable instance\")" << endl
Expand Down Expand Up @@ -986,22 +994,22 @@ void t_py_generator::generate_py_struct_reader(ostream& out, t_struct* tstruct)
const vector<t_field*>& fields = tstruct->get_members();
vector<t_field*>::const_iterator f_iter;

if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
out << indent() << "@classmethod" << endl << indent() << "def read(cls, iprot):" << endl;
} else {
indent(out) << "def read(self, iprot):" << endl;
}
indent_up();

const char* id = is_immutable(tstruct) ? "cls" : "self";
const char* id = is_immutable(tstruct, gen_immutable_exc_) ? "cls" : "self";

indent(out) << "if iprot._fast_decode is not None "
"and isinstance(iprot.trans, TTransport.CReadableTransport) "
"and "
<< id << ".thrift_spec is not None:" << endl;
indent_up();

if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
indent(out) << "return iprot._fast_decode(None, iprot, [cls, cls.thrift_spec])" << endl;
} else {
indent(out) << "iprot._fast_decode(self, iprot, [self.__class__, self.thrift_spec])" << endl;
Expand All @@ -1011,7 +1019,7 @@ void t_py_generator::generate_py_struct_reader(ostream& out, t_struct* tstruct)

indent(out) << "iprot.readStructBegin()" << endl;

if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
t_field* tfield = *f_iter;
std::ostringstream result;
Expand Down Expand Up @@ -1053,7 +1061,7 @@ void t_py_generator::generate_py_struct_reader(ostream& out, t_struct* tstruct)
indent_up();
indent(out) << "if ftype == " << type_to_enum((*f_iter)->get_type()) << ":" << endl;
indent_up();
if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
generate_deserialize_field(out, *f_iter);
} else {
generate_deserialize_field(out, *f_iter, "self.");
Expand All @@ -1073,7 +1081,7 @@ void t_py_generator::generate_py_struct_reader(ostream& out, t_struct* tstruct)

indent(out) << "iprot.readStructEnd()" << endl;

if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
indent(out) << "return cls(" << endl;
indent_up();
for (f_iter = fields.begin(); f_iter != fields.end(); ++f_iter) {
Expand Down Expand Up @@ -2303,7 +2311,7 @@ void t_py_generator::generate_deserialize_field(ostream& out,
* Generates an unserializer for a struct, calling read()
*/
void t_py_generator::generate_deserialize_struct(ostream& out, t_struct* tstruct, string prefix) {
if (is_immutable(tstruct)) {
if (is_immutable(tstruct, gen_immutable_exc_)) {
out << indent() << prefix << " = " << type_name(tstruct) << ".read(iprot)" << endl;
} else {
out << indent() << prefix << " = " << type_name(tstruct) << "()" << endl
Expand Down Expand Up @@ -2358,16 +2366,16 @@ void t_py_generator::generate_deserialize_container(ostream& out, t_type* ttype,
// Read container end
if (ttype->is_map()) {
indent(out) << "iprot.readMapEnd()" << endl;
if (is_immutable(ttype)) {
if (is_immutable(ttype, gen_immutable_exc_)) {
indent(out) << prefix << " = TFrozenDict(" << prefix << ")" << endl;
}
} else if (ttype->is_set()) {
indent(out) << "iprot.readSetEnd()" << endl;
if (is_immutable(ttype)) {
if (is_immutable(ttype, gen_immutable_exc_)) {
indent(out) << prefix << " = frozenset(" << prefix << ")" << endl;
}
} else if (ttype->is_list()) {
if (is_immutable(ttype)) {
if (is_immutable(ttype, gen_immutable_exc_)) {
indent(out) << prefix << " = tuple(" << prefix << ")" << endl;
}
indent(out) << "iprot.readListEnd()" << endl;
Expand Down Expand Up @@ -2806,17 +2814,17 @@ string t_py_generator::type_to_spec_args(t_type* ttype) {
+ type_to_spec_args(((t_map*)ttype)->get_key_type()) + ", "
+ type_to_enum(((t_map*)ttype)->get_val_type()) + ", "
+ type_to_spec_args(((t_map*)ttype)->get_val_type()) + ", "
+ (is_immutable(ttype) ? "True" : "False") + ")";
+ (is_immutable(ttype, gen_immutable_exc_) ? "True" : "False") + ")";

} else if (ttype->is_set()) {
return "(" + type_to_enum(((t_set*)ttype)->get_elem_type()) + ", "
+ type_to_spec_args(((t_set*)ttype)->get_elem_type()) + ", "
+ (is_immutable(ttype) ? "True" : "False") + ")";
+ (is_immutable(ttype, gen_immutable_exc_) ? "True" : "False") + ")";

} else if (ttype->is_list()) {
return "(" + type_to_enum(((t_list*)ttype)->get_elem_type()) + ", "
+ type_to_spec_args(((t_list*)ttype)->get_elem_type()) + ", "
+ (is_immutable(ttype) ? "True" : "False") + ")";
+ (is_immutable(ttype, gen_immutable_exc_) ? "True" : "False") + ")";
}

throw "INVALID TYPE IN type_to_spec_args: " + ttype->get_name();
Expand Down

0 comments on commit 402f505

Please sign in to comment.