ChuanqiXu added a comment. In D138859#3958958 <https://reviews.llvm.org/D138859#3958958>, @vsapsai wrote:
> In D138859#3955806 <https://reviews.llvm.org/D138859#3955806>, @ChuanqiXu > wrote: > >> And I think we should add tests for this. e.g., in the ASTImporterTests.cpp. > > I'm not sure which test would be useful in this case. We kinda already test > that different modules agree which attributes should be ODR hashable. That is > enforced by detecting attribute mismatches in odr_hash.cpp (if attributes > aren't hashable - no mismatches). Yeah, I admit it is redundant from some perspective. I think the lit tests are different with the unittest. The lit tests check if the end behavior is expected. And the unittest tests checks if the status of the attribute is expected. For example, it is possible to discard the changes in this patch and keep these lit tests passes. I think more tests are always good. Especially now it is not hard to add these tests. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138859/new/ https://reviews.llvm.org/D138859 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
