Equivalent overloads already exist for makeArrayRef.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/unittests/ADT/MutableArrayRefTest.cpp | ||
---|---|---|
19 ↗ | (On Diff #432120) | You may reuse ArrayRefTest.cpp since we use ArrayRef.h instead of MutableArrayRef.h. There should be a test checking that the element is mutable. |
Address reviews:
- fix comments
- moved to ArrayRefTest.cpp
- add a check that the mutable array ref elements can be assigned to
llvm/include/llvm/ADT/ArrayRef.h | ||
---|---|---|
549 | Adding a comment for each makeMutableArrayRef overload doesn't add much value to me. The argument types are obvious. I think having one comment covering all the overloads suffices. |
Ping -- are there any substantial comments left? Based on previous feedback this is probably good to go with the changes I made last week?
There are two comments which are "Not Done". You need to address them and click "Done".
llvm/include/llvm/ADT/ArrayRef.h | ||
---|---|---|
549 | This just feels like a matter of taste. The patch as-is follows the existing convention in the file. |
Missing doc comment?