erichkeane accepted this revision.
erichkeane added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/lib/Sema/SemaType.cpp:5949
assert(!T.isNull() && "T must not be null at the end of this function");
- if (D.isInvalidType())
+ if (!AreDeclaratorChunksValid)
return Context.getTrivialTypeSourceInfo(T);
----------------
ilya-biryukov wrote:
> erichkeane wrote:
> > ilya-biryukov wrote:
> > > erichkeane wrote:
> > > > Shouldn't the `D.setInvalidType(true)` in each of the branches work
> > > > here? So this variable is unnecessary? Else this is a good change
> > > > IMO.
> > > (Let me know if I'm misreading the suggestion, I was not sure if my
> > > understanding is correct).
> > > If we call `setInvalidType` more, we would actually get more crashes as
> > > the problematic branch is the one that calls `getTrivialTypeSourceInfo`.
> > > To avoid the extra variable, we could instead ensure that the type always
> > > gets replaced with something trivial `T = Context.IntTy`. But I didn't
> > > want to go this path because this causes worse error recovery (going from
> > > something like `void(<recovered-to:int>)` to `<recovered-to:int>`) and
> > > possibly correctness issues (e.g. will we start getting function-decls or
> > > lambdas that do not have function types and other assertions may fire).
> > My suggestion was that `setInvalidType` is called everywhere that
> > `AreDeclatorChunksValid` above (sans 1 perhaps?). So my question was
> > whether this introduced variable was necessary.
> Ah, thanks for clarifying the question.
> `setInvalidType` is called in much more places and the point of this patch is
> to capture a subcategory of invalid types that still follow the declarator
> structure.
>
Got it, thanks for the clarification. I'm OK with this as-is, then.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D146971/new/
https://reviews.llvm.org/D146971
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
