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

Kadir Cetinkaya via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed May 3 07:56:18 PDT 2023


kadircet added a comment.

In D149733#4315360 <https://reviews.llvm.org/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 type as soon as it escapes the function responsible for 'making' it (that is, the one that calls the ::Create and is filling in everything).

I guess that's where the issue comes from. In theory `Parser::ParseLambdaExpressionAfterIntroducer` calls both `Sema::ActOnLambdaExpressionAfterIntroducer` (which creates incomplete method decl) and `Sema::ActOnStartOfLambdaDefinition` (which populates type info), so once we exit the parse function the methoddecl should have a valid type.
But the way clang implements code completion breaks that guarantee, as parser can hand out those semi-complete declarations to code completion consumers if code completion point is reached while parsing the tokens in between.
More importantly I don't know if this "variant" holds in all the places that creates a function decl (even in this example we can actually take an early exit <https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseExprCXX.cpp#L1337> but we at least mark the decl invalid before doing so).

> If that patch added a path that doesn't set the return type, perhaps we should just fix that?

it's not just the return type, it's the full type for the function, see here <https://reviews.llvm.org/differential/changeset/?ref=3973082#:~:text=QualType()%2C%20/*Tinfo%3D*/nullptr%2C%20SC_None%2C> (line 919 of clang/lib/Sema/SemaLambda.cpp in case the link doesn't work).
due to above mentioned reasons, i don't think there's anything wrong with that patch in particular (either it's fine or we've a bunch more cases that are wrong and hard to fix due to the first reason you mentioned), it just separated introduction and finalisation of a decl. https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaDeclCXX.cpp#L14469 is another example (and we've got bunch more).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D149733/new/

https://reviews.llvm.org/D149733



More information about the cfe-commits mailing list