Add FieldInfo::field_type_name

[workingjubilee]

In my case, I want to make a field private and wrap it behind an accessor function that makes the field unsafe to access. I need the field's type in order to make this decision correctly. In some cases a type name is not available and I am fine with that.

[pvdrz]

Is there any chance you could document this in the changelog? it looks good to me otherwise

[emilio]

IT seems we should probably include c++ namespaces and so on here? So, use the same name as all the blocklists etc use.

[workingjubilee]

uhhh can I be counseled on how that should be done because I am not very familiar with this codebase, much less C++

[workingjubilee]

I also should note that I specifically want None instead of the unnamed at file... output that currently is given in certain other cases, because I think that output is pointless, even harmful: it requires me to filter-by-string for anonymous types, which is much more error-prone and dependent on the vagaries of bindgen's output instead of using a type encoding.

[pvdrz]

@emilio do we include c++ namespaces for the type name as well? otherwise it would be a bit inconsistent.

[emilio]

@pvdrz type_name currently uses canonical_name, so yes, I think so.

[workingjubilee]

I don't really need the canonical name, though, because I don't need ExprEvalStep__bindgen_ty_1, because what's important to me is that it is an anonymous structure, and I don't necessarily want to reason about anonymous structures by their name. Maybe their other attributes, but the real answer for their name is the empty string.

[emilio]

Well, sure, but my point is, if you have:

namespace foo {
  class Bar { ... };
}
namespace baz {
  class Bar { ... };
}

You want to be able to differentiate between one and the other right? btw, it's unclear to me what this does for pointer fields, templates, and so on. I assume the answer is reasonable, but still it'd be worth having some tests.

You might still can make the anonymous / non-anonymous distinction (if thing.name().is_some() { Some(thing.canonical_name()) } else { None } or same using map or what not)

[workingjubilee]

My answer is that I don't really care about whatever the answer is to that because I don't use C++, not personally, nor in a work context, and in the one case where I've tried to use bindgen against it in anger, its insistence on qualifying namespaces was actually a disadvantage as the namespaces were effectively meaningless and I would have been better off if the Rust code did not know about them (they were used as a crude implementation of parameterized modules, instead of using something sensible for this facility, like e.g. template parameters).

[pvdrz]

To be honest, I'd rather see this working in C and sort of working in C++ that not seeing it working at all. I think it wouldn't be unreasonable to take this as it is and see if someone else actually ends up using this and requiring the canonical_name as we could either add it as a new field or use it instead of the regular name if a flag is enabled.

@workingjubilee @emilio does that works for both of you?

[workingjubilee]

Sure.

And I'd be happy to do the .map-based solution, but my main point of confusion at this juncture is in this code:

rust-bindgen/bindgen/ir/item.rs

Lines 40 to 68 in 2fb25e3

	pub(crate) trait ItemCanonicalName {
	/// Get the canonical name for this item.
	fn canonical_name(&self, ctx: &BindgenContext) -> String;
	}
	/// The same, but specifies the path that needs to be followed to reach an item.
	///
	/// To contrast with canonical_name, here's an example:
	///
	/// ```c++
	/// namespace foo {
	/// const BAR = 3;
	/// }
	/// ```
	///
	/// For bar, the canonical path is `vec!["foo", "BAR"]`, while the canonical
	/// name is just `"BAR"`.
	pub(crate) trait ItemCanonicalPath {
	/// Get the namespace-aware canonical path for this item. This means that if
	/// namespaces are disabled, you'll get a single item, and otherwise you get
	/// the whole path.
	fn namespace_aware_canonical_path(
	&self,
	ctx: &BindgenContext,
	) -> Vec<String>;
	/// Get the canonical path for this item.
	fn canonical_path(&self, ctx: &BindgenContext) -> Vec<String>;
	}

With "namespace-aware canonical paths" being a thing, this suggests it's more complicated than just calling canonical_name(), so I feel, basically, not qualified to evaluate what "canonical name" means in bindgen because it conflates C++ namespaces with generated symbols.

[bors-servo]

☔ The latest upstream changes (presumably 9a8e5ca) made this pull request unmergeable. Please resolve the merge conflicts.