Sirraide wrote: > I am not sure removing const everywhere for one usage is a net-positive. > There are no reason for template arguments to ever be modified.
I can’t say I know the full scope of this refactor (as in, what it would actually take to remove all of these `const_cast`s for good, because if I understand this correctly, this is only supposed to be the first step here because it ends up adding more `const_cast`s than it removes, from what I can tell), but I guess my two cents is that, while that statement makes sense to me, my understanding is that e.g. AST nodes in general aren’t really *supposed* to be modified after creation, but we have to do that in a bunch of places anyway. This just seems like another example of that. I’m not sure keeping `const` everywhere and pretending like we don’t do that, only to then happily `const_cast` it away whenever it turns out that, actually, we *do* need to modify this thing after all, is really that helpful. Does `const` really mean anything at that point if, in practice, it ends up being used to signify that ‘this API *probably* doesn’t modify this thing, unless it has to, in which case it does’? I’m sure that’s not the case in every place we use `const`, but I’ve seen it often enough to where whenever I now see `const` somewhere on a function parameter in Clang, I’m never quite sure whether that function *actually* doesn’t modify that argument, or whether it’s just `const` because the argument just happens to be `const` everywhere we call that function. > I would like to see explored a solution that clones > MultiLevelTemplateArgumentList instead of mutating it in place I might be missing a bit of the context here, but wouldn’t that have a performance impact? But, yeah, I suppose, as you said, someone would probably have to benchmark that. https://github.com/llvm/llvm-project/pull/104687 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
