Yeah, this patch is interesting. I'm not quite sure what to think of it. So some scattered thoughts below:
* Fundamentally I think most (though not all) of these comments are obvious and unnecessary. But if they were that obvious, then we didn't need the comments in the first place, so I clearly need to adjust my expectations at least somewhat.
* Looking at something like hlsl_type: intuitively I'd think it's pretty easy to guess *what* this is; it's a representation of a data type. It's also probably not hard to figure out what the contents of the type are just from the name, although some of them probably require an understanding of the HLSL language first.
* More interesting are the questions that I think are *not* obvious just from looking at the source: is it source-level, or something further down? Is it unique? Where are all of the data types stored? [The answers, of course, being: yes, it's source-level; no, it's not unique; data types are stored in a list in hlsl_ctx.types].
* There's also questions like: where are types constructed, and when are they retrieved? Same for individual fields—I can guess that the fields like base_type/dimx/dimy are set upon creation, but when is reg_size or bytecode_offset set and used?
* Figuring out what kind of questions like this need to be asked is probably hard, though, and anticipating every question even harder. I'm basically trying to think of the kind of questions I've asked when I encounter a new project.
* I think Giovanni kind of hit the nail on the head with one of the above comments—it's pretty obvious what sampler_dim means, the more interesting question is when is it set.
* Then there's the awkward fact that a lot of the stuff that needs documentation should probably go away, like reg_size. (Coincidence?)
* Some fields, like hlsl_ctx.profile, are not exactly clear just from the name and type what they are (because naming is hard and can only be so specific). But if you follow the chain to look at the contents of struct hlsl_profile_info, it becomes clearer. Is it still worth documenting hlsl_ctx.profile in that case? I don't know, maybe...
* On the other hand, there's also stuff like hlsl_deref, which I think is a much less intuitive construction. I.e. I think it's clear *what* hlsl_type is, at least, but hlsl_deref is a bit more idiosyncratic. Same with hlsl_src (which is basically a node reference plus an entry in a def-use chain). So, y'know, not everything is obvious.
I'll probably set this aside for a few days and keep thinking about it, unless of course we end up having more discussion in that time ;-)