Gotta land some of this stuff... #539
Conversation
The all_template_parameters method gets the complete set of template parameters that can affect a given item. Note that this item doesn't need to be a template declaration itself for `Some` to be returned from this method (in contrast to `self_template_params`). If this item is a member of a template declaration, then the parent's template parameters are included here. See the example in TemplateDeclaration's doc comment for details.
Rather than determining whether any given template parameter is used at all globally, determine the set of template parameters used by any given IR node.
| // Rc<RefCell<ItemSet>> to make the borrow checker happy, but it | ||
| // isn't clear that the added indirection wouldn't outweigh the cost | ||
| // of malloc'ing a new ItemSet here. Ideally, `HashMap` would have a | ||
| // `split_entries` method analogous to `slice::split_at_mut`... |
There was a problem hiding this comment.
I am fixing this unnecessary malloc thrashing in a follow up, before we turn this analysis on by default.
| if each_self_params.is_empty() { | ||
| None | ||
| } else { | ||
| Some(each_self_params.into_iter() |
| /// |Decl. | self_template_params | num_self_template_params | all_template_parameters| | ||
| /// +------+----------------------+--------------------------+------------------------+ | ||
| /// |Foo | Some([T, U]) | Some(2) | Some([T, U]) | | ||
| /// |Bar | Some([V]) | Some(1) | Some([T, U, V]) | |
There was a problem hiding this comment.
This made me think... Do we handle shadowing? (Not that is a blocker, nor that we do now, but if we did that'd be awesome :P)
There was a problem hiding this comment.
I'll make a note to add a test.
| for (arg, param) in args.iter().zip(params.iter()) { | ||
| if self.used[&decl].contains(param) { | ||
| if self.ctx.resolve_item(*arg).is_named() { | ||
| self.used.get_mut(&id).unwrap().insert(*arg); |
There was a problem hiding this comment.
i believe you should be able to use the self.used[&id].insert(*arg) syntax here too?
There was a problem hiding this comment.
It was originally like that, but for some reason type inference was inferring an immutable indexing operation, rather than a mutable one.
/me shrugs
| let new_size = self.used.len(); | ||
| new_size != original_size | ||
| let new_len = self.used[&id].len(); | ||
| assert!(new_len >= original_len); |
There was a problem hiding this comment.
Well, I'd be really scared if this didn't hold, but better safe than sorry.
There was a problem hiding this comment.
Yep, just a sanity check, more for future modifications than for the code as it is now.
| //! | ||
| //! A monotone function `f` is a function where if `x <= y`, then it holds that | ||
| //! `f(x) <= f(y)`. It should be clear that running a monotone function to a | ||
| //! fix-point on a finite lattice will always terminate: `f` can only "move" |
There was a problem hiding this comment.
+1000 for the comment/code ratio, this is awesome :)
|
@bors-servo r=emilio |
|
📌 Commit 7e22fca has been approved by |
Gotta land some of this stuff... My patch queue is getting quite large, as I refactor templates in bindgen, so I figure I should land the stuff that is in good shape sooner rather than later :-P This stuff tweaks the named template parameter analysis, adds some edge kinds, stuff like that. This is still not used by codegen, yet. I have another branch where I de-dupe named types and all of that, and there is only two tests failing now, but still some clean up needed. This also contains the expanded documentation for te named template parameter analysis that I promised. See each commit message for details. r? @emilio
|
☀️ Test successful - status-travis |
My patch queue is getting quite large, as I refactor templates in bindgen, so I figure I should land the stuff that is in good shape sooner rather than later :-P
This stuff tweaks the named template parameter analysis, adds some edge kinds, stuff like that. This is still not used by codegen, yet. I have another branch where I de-dupe named types and all of that, and there is only two tests failing now, but still some clean up needed.
This also contains the expanded documentation for te named template parameter analysis that I promised.
See each commit message for details.
r? @emilio