mizvekov added a comment. In D112374#3650749 <https://reviews.llvm.org/D112374#3650749>, @glandium wrote:
> FWIW, this change also broke this check in Firefox's clang plugin: > https://searchfox.org/mozilla-central/rev/0d11f3660945ce35c49501bb44bc4f82bb2b503c/build/clang-plugin/NoPrincipalGetURI.cpp Thanks for the report! I think this is a combination of 1) and 2) from above post (https://reviews.llvm.org/D112374#3650056). I believe that hasType(asString()) is supposed to match the type as-written. The fact that it did not so for unqualified names is what this patch is fixing. Can you confirm that this `NoPrincipalGetURI` matcher would, in clang version without this patch here, fail to match if you had rewritten the `GetUri` method to use a qualified name? Something like this: From: nsIPrincipal *getURI(); To: ::nsIPrincipal *getURI(); Ie qualify with the global namespace or something equivalent that works in the Firefox codebase? I believe that matching to the canonical type would be the simplest fix for that bug: On mozilla-central/build/clang-plugin/NoPrincipalGetURI.cpp change from: anyOf(on(hasType(asString("class nsIPrincipal *"))), on(hasType(asString("class nsIPrincipal")))), To: anyOf(on(hasType(hasCanonicalType(asString("class nsIPrincipal *")))), on(hasType(hasCanonicalType(asString("class nsIPrincipal"))))), Ie nest the `asString` matcher inside a `hasCanonicalType` matcher. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112374/new/ https://reviews.llvm.org/D112374 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
