sammccall added a comment. In D115106#3218000 <https://reviews.llvm.org/D115106#3218000>, @aaron.ballman wrote:
>> I think `anyRedeclaration(functionDecl(isStaticStorageClass))`is too >> difficult to get right and too easy to get wrong for such a common/simple >> concept. > > Get right in terms of implementation, or in terms of use, or both? Use, if I'm understanding your question. If we don't expose this as a matcher with some name, it's a recipe the person writing the matcher has to remember. There's no good way to look it up, and it's easy to write something that seems good but isn't. (This problem is pervasive with AST and matchers, and this isn't the worst example.) >> I know what a static method is, but I would have made the same mistake here. >> In general asking people to describe syntax to answer a semantic question >> invites this. > > Because our AST is intended to reflect the syntax, our AST matchers kind of > *do* match syntax -- I think it's the reason we're in the problem (and this > is not the first time it's come up). The clang AST is the best tool we have to describe syntax *and* to describe semantics. Maybe by design it captures "all" syntax and "enough" semantics, but users reasonably care less about the design than about their alternatives and problems. The clang AST is useful for answering syntax questions (better than regex!) but it's _irreplaceable_ for answering semantics questions (better than... err?). So we have syntax and semantics in one namespace, and static is the worst because it means like 3 syntactic and 5 semantic things :-( > There's a cognitive disconnect between the AST matchers traversing the AST > and applying a match on all node and the C & C++ concept of redeclarations. Definitely, this highlights the disconnect really well. And some of the accessors on a redecl will delegate to the canonical while others won't. (Can we call this "syntax semantics" vs "semantics semantics"?!) I definitely think we *should* have a redecl matcher - I just don't think we should make users use it to answer common semantic questions. That leaves matcher authors remembering and reciting the rules about how syntax aggregates over redecls to produce semantics. We'll make mistakes. >> Here's a hack: CXXMethodDecl has both `isStatic()` and `isInstance()`. I >> agree that `isStatic` is a risky name, but I don't think `isInstance` is. >> Can we just spell the matcher you want `not(isInstance())`? > > This could work, but I don't think it's particularly satisfying in situations > that aren't binary. Oh, I totally agree. I just meant specifically for this case, as a way to avoid the dilemma of ambiguous vs unwieldy. > This could work, but I don't think it's particularly satisfying in situations > that aren't binary. Like asking "does this have static storage duration" (as > opposed to thread, automatic, or dynamic storage duration). And it turns out > that the behavior there is opposite to asking about the storage class > specifier: https://godbolt.org/z/hqserfxzs I'm not totally surprised by that, because "storage duration" sounds like a semantic property to me, while "storage class" (specifier) is a syntactic feature. But it's not a distinction I'd pick up on if I wasn't looking for it, and different people are going to read things different ways. My favorite naming convention I've seen in the AST is e.g. `getBlob()` vs `getBlobAsWritten()`. Maybe `getEffectiveBlob()` would be even clearer for the semantic version. But I'm not sure how much it's possible to realign matcher names around that sort of thing if the AST names aren't going to change. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115106/new/ https://reviews.llvm.org/D115106 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
