Marking declarations as invalid

Hi,

Would someone mind explaining when (or when not) to mark a declaration, i.e. FunctionDecl or VarDecl, an invalid declaration?

Then, if the declaration is marked invalid, at what do we look at/use that information?

Thanks,

Nate

Hello,

I wanted to follow up to see if anyone had any thoughts about this. I’d appreciate it.

Thanks!

I’ll turn the question around.

What are you trying to accomplish? Do you see a case where you would like to mark a declaration as invalid, but don’t know the right way? Or do you want to do something extra in the cases that a declaration has been found as invalid? The context may spur the right kind of conversation.

Employee of Qualcomm Innovation Center, Inc.

Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project

I’m not trying to accomplish anything at the moment per se. Just make sure I don’t run into trouble, and where I would run into trouble by incorrectly marking a FunctionDecl (or something else) as invalid.

For example, I have a check which may emit a diagnostic (SemaDecl.cpp current lines 7595:7600):

if (isConstexpr) {
Diag(D.getDeclSpec().getConstexprSpecLoc(),
diag::err_concept_decl_invalid_specifiers)
<< 1 << 3;
NewFD->setInvalidDecl(true);
}

Now, is it (un)necessary to mark the FunctionDecl as invalid here?

Below that check, at line 7865, we check to see if the declaration is invalid which might get me into trouble when looking at a function template specialization.

Hope that gives a little more context.

Here are some guidelines:

  * You must not mark a declaration as invalid unless you have emitted an
error.
  * If you emitted an error with a fix-it hint, you should not mark the
declaration as invalid (and you should recover as if the fix-it hint were
applied). We would like this to be a hard rule, but there are existing
cases where we don't follow this.
  * You should mark a declaration as invalid if you think it's sufficiently
far from the user's intent that follow-on diagnostics should be suppressed.
  * You must mark a declaration as invalid if it does not correspond to
anything like a valid declaration (if it would be unreasonable to expect
downstream code to do the right thing with it).

Thanks for this Richard. That helps a lot.

So, I could (should?) have used a fix-it hint in the example above and not mark the declaration invalid, correct? (Aside from your third bullet.)

Would you be able to give an example of the fourth bullet and/or point to where the downstream code would do the right thing?

>
>>
>> Hi,
>>
>> Would someone mind explaining when (or when not) to mark a declaration,
i.e. FunctionDecl or VarDecl, an invalid declaration?
>>
>> Then, if the declaration is marked invalid, at what do we look at/use
that information?
>
> Here are some guidelines:
>
> * You must not mark a declaration as invalid unless you have emitted
an error.
> * If you emitted an error with a fix-it hint, you should not mark the
declaration as invalid (and you should recover as if the fix-it hint were
applied). We would like this to be a hard rule, but there are existing
cases where we don't follow this.
> * You should mark a declaration as invalid if you think it's
sufficiently far from the user's intent that follow-on diagnostics should
be suppressed.
> * You must mark a declaration as invalid if it does not correspond to
anything like a valid declaration (if it would be unreasonable to expect
downstream code to do the right thing with it).

Thanks for this Richard. That helps a lot.

So, I could (should?) have used a fix-it hint in the example above and not
mark the declaration invalid, correct? (Aside from your third bullet.)

Ah, well... you should only put a fix-it hint on an error when you are
extremely confident you know what the user did wrong. People find it very
frustrating when we incorrectly guess what they did wrong, and then produce
follow-on diagnostics about our bad guess.

Would you be able to give an example of the fourth bullet and/or point to
where the downstream code would do the right thing?

A variable with type 'void', or a redeclaration of a variable with a
different type. You can think about it this way: when you're writing code
that looks at an AST node, it's reasonable for you to assume that the AST
node corresponds to some part of the underlying language (unless it's
marked as invalid), so you don't need to re-check properties that the
language itself guarantees (such as, all declarations of a function have a
type that is in fact a function type). And this puts a requirement on code
that creates AST nodes, to mark them as invalid if it cannot satisfy that
assumption.

>
>>
>> Hi,
>>
>> Would someone mind explaining when (or when not) to mark a
declaration, i.e. FunctionDecl or VarDecl, an invalid declaration?
>>
>> Then, if the declaration is marked invalid, at what do we look at/use
that information?
>
> Here are some guidelines:
>
> * You must not mark a declaration as invalid unless you have emitted
an error.
> * If you emitted an error with a fix-it hint, you should not mark the
declaration as invalid (and you should recover as if the fix-it hint were
applied). We would like this to be a hard rule, but there are existing
cases where we don't follow this.
> * You should mark a declaration as invalid if you think it's
sufficiently far from the user's intent that follow-on diagnostics should
be suppressed.
> * You must mark a declaration as invalid if it does not correspond to
anything like a valid declaration (if it would be unreasonable to expect
downstream code to do the right thing with it).

Thanks for this Richard. That helps a lot.

So, I could (should?) have used a fix-it hint in the example above and
not mark the declaration invalid, correct? (Aside from your third bullet.)

Ah, well... you should only put a fix-it hint on an error when you are
extremely confident you know what the user did wrong. People find it very
frustrating when we incorrectly guess what they did wrong, and then produce
follow-on diagnostics about our bad guess.

Ahh, yeah, that makes sense. Understandable as well.

Would you be able to give an example of the fourth bullet and/or point to

where the downstream code would do the right thing?

A variable with type 'void', or a redeclaration of a variable with a
different type. You can think about it this way: when you're writing code
that looks at an AST node, it's reasonable for you to assume that the AST
node corresponds to some part of the underlying language (unless it's
marked as invalid), so you don't need to re-check properties that the
language itself guarantees (such as, all declarations of a function have a
type that is in fact a function type). And this puts a requirement on code
that creates AST nodes, to mark them as invalid if it cannot satisfy that
assumption.

I see. Thanks for the explanation (and follows from the first three bullets
you gave).