dblaikie added a comment.

In D139379#3972876 <https://reviews.llvm.org/D139379#3972876>, @ayermolo wrote:

> In D139379#3972871 <https://reviews.llvm.org/D139379#3972871>, @dblaikie 
> wrote:
>
>> Perhaps the change to use accessors could be removed, now that you've used 
>> it to find all the places that needed to be fixed up? (like just using it 
>> for cleanup/temporary purposes, without needing to commit that API change?)
>
> I am not sure what other projects are using it, that I missed, or not in 
> llvm-trunk, but are based of it.

It's awkward to convolute the API to ensure a breakage - I think it's best to 
leave it as-is, for the most part. I guess you needed the 32 bit accessors so 
existing code doesn't become UB because of truncation to 32 bit?

Could the code keep the existing member names, provide the wrappers without 
turning the members into an array and needing named index constants, etc, at 
least? (though even then, seems like there's more to it than needed)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139379/new/

https://reviews.llvm.org/D139379

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to