apache / tvm

Open deep learning compiler stack for cpu, gpu and specialized accelerators
https://tvm.apache.org/
Apache License 2.0
11.41k stars 3.4k forks source link

[QoL][IR] Provide default constructor for NameSupply/GlobalVarSupply #17135

Open Lunderberg opened 2 days ago

Lunderberg commented 2 days ago

Prior to this commit, a tvm::NameSupply needed to be constructed with an explicit const String& prefix argument. Omitting this argument would fall back to the default constructor provided by the TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS macro, producing a NameSupply holding a nullptr. This then leads to a segfault when the null NameSupply is used.

The vast majority of usages of NameSupply::NameSupply (29 out of 31) initialize it with an empty prefix string. The remaining two use cases initialize it with a non-empty prefix string. There are no cases in which a null NameSupply is initialized.

This commit updates NameSupply to use the TVM_DEFINE_MUTABLE_NOTNULLABLE_OBJECT_REF_METHODS macro instead of TVM_DEFINE_MUTABLE_OBJECT_REF_METHODS. This allows the default constructor to provide the common usage of a NameSupply with an empty prefix, rather than the error-prone usage of a null NameSupply

A similar change is also made for GlobalVarSupply, as the majority of its uses also default to an empty prefix (11 out of 13).

Lunderberg commented 2 days ago

I've accidentally made this mistake too many times, and wanted to make it harder to hit this segfault.