[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

aaron.ballman added a comment. In D149733#4342705 , @kadircet wrote: > In D149733#4342300 , @aaron.ballman > wrote: > >> But at the same time, it's become more obvious (at least to me) that clangd >> has feature

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG0d1be98a67e2: [clang][USR] Prevent crashes on incomplete FunctionDecls (authored by kadircet). Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

kadircet added a comment. In D149733#4342300 , @aaron.ballman wrote: > But at the same time, it's become more obvious (at least to me) that clangd > has features that don't work with all of the invariants in Clang and I don't > know that we ever really

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

aaron.ballman accepted this revision. aaron.ballman added a comment. This revision is now accepted and ready to land. In D149733#4342095 , @kadircet wrote: >> I think we probably should have a broader discussion before moving forward >> here. It's not th

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

kadircet added a comment. > I think we probably should have a broader discussion before moving forward > here. It's not that this isn't incremental progress fixing an issue, but it's > more that this same justification works to add the workaround 200 more times > without ever addressing the und

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

aaron.ballman added a comment. In D149733#4321610 , @kadircet wrote: >> However, I'm not keen on us playing whack-a-mole with the kinds of checks >> from this review. For starters, that's going to have a long-tail that makes >> it hard to know if we've

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

kadircet added a comment. gentle ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149733/new/ https://reviews.llvm.org/D149733 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

kadircet added a comment. > However, I'm not keen on us playing whack-a-mole with the kinds of checks > from this review. For starters, that's going to have a long-tail that makes > it hard to know if we've ever gotten to the bottom of the issue. But also, > each one of these checks is basicall

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

aaron.ballman added subscribers: v.g.vassilev, junaire, rsmith. aaron.ballman added a comment. Adding in some of the clang-repl folks because they are likely to run into this as well, and adding Richard in case he has more historical context. What I understand of the historical context here is t

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

kadircet added a comment. In D149733#4315360 , @erichkeane wrote: > I don't recall the case of a Null type being valid for functions (perhaps > Aaron does? Or perhaps its an error condition?). But otherwise, I would > expect `FunctionDecl` to have a

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

erichkeane added a comment. Hmm... So I don't see us being able to change how we create `FunctionDecl`s, the way we are doing it with a 'build up to the final thing' is about all we can do. However, it should never escape its 'creation' functions without its type set. IF I were to design this

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

kadircet added a comment. FWIW, https://github.com/llvm/llvm-project/blob/main/clang/lib/AST/Decl.cpp#L2987 is the assertion that suggests "null type is fine for functiondecls", but as pointed out pretty much all of the methods assume it's non-null && most of the users also don't check for val

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

ilya-biryukov added a subscriber: erichkeane. ilya-biryukov added a comment. It's true that `FunctionType` having a null type does break a lot of even its own methods, e.g. even something as simple as `isVariadic` will dereference a null pointer as it uses `getType()->getAs()`. I am not entirel

[PATCH] D149733: [clang][USR] Prevent crashes on incomplete FunctionDecls

kadircet created this revision. kadircet added reviewers: aaron.ballman, ilya-biryukov. Herald added a subscriber: arphaman. Herald added a project: All. kadircet requested review of this revision. Herald added projects: clang, clang-tools-extra. Herald added a subscriber: cfe-commits. FunctionDec