[PATCH] D138859: [ODRHash] Drive attribute hashing through TableGen. NFC intended.

Chuanqi Xu via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 29 19:13:44 PST 2022


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



More information about the cfe-commits mailing list