https://github.com/NagyDonat commented:
Overall LPGTM (looks plausibly good to me), but I'm not familiar enough with the intricacies of template specializations to verify whether the logic is correct. I added some stylistic nitpicks, mostly about the use of `auto`. I didn't read the testcases yet because their sheer amount is daunting and there are no delimiters and very few explanatory comments among them. If I understand it correctly the significant entrypoints are the functions named `instantiate_...`, but this is not immediately obvious -- it would be nice to have clear separators (e.g. a line of 80 slashes) after each such function. Also, I would be grateful to have a few comment lines describing the significance of each testcase -- these should appear just below the delimiter line to help with understanding the code that follows them. I would prefer if you wrote these descriptions without AI use, because I don't trust AI summaries and I highly trust that you would highlight the most relevant factors -- but you have more experience with claude, so I can accept it if you are confident in it. By the way, if you see any redundant testcases, it would be nice to remove them -- in this situation I feel that less is more. Although I'm usually good with abstractions, these testcases are so abstract and similar, that they blend together in my mind and interfere with understanding each other. https://github.com/llvm/llvm-project/pull/183727 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
