Patch adding a Boolean flag to TagDecl nodes.

Hello.

The attached patch adds a new Boolean flag to TagDecl nodes.

The new flag, named IsDefinedInDeclarator, tells whether or not the
TagDecl is a definition that was syntactically occurring (possibly via
macro expansions) in a declarator. Examples include tag types defined in
function return types, function parameter types, variable or field
declarations and type arguments of cast or sizeof expressions.

The new flag is meant to help clients such as code style checkers.

Cheers,
Enea Zaffanella.

TagDefinedInDeclarator.patch (3.15 KB)

Any comment on the proposed patch?

Thanks in advance,
Enea Zaffanella.

Enea Zaffanella wrote:

This feels like the wrong solution. I'd like to hear from others, but the best approach seems to be to extend TagTypeLoc with a bit that indicates whether this use of the tag type was a reference or a definition. That way, a pretty-printer could know exactly where to put the definition.

  - Doug

Our aim is exactly that and it works perfectly using the proposed patch:
the RecordDecl inserted in DeclContext parsing

typedef struct s { int a; } x;

is marked with IsDefinedInDeclarator bit and then can be excluded from
pretty printer output. Next, analyzing TypedefDecl it could be easily
verified that the location of TypedefDecl is the same of RecordDecl and
then we can know easily that the record definition was embedded in the
typedef declaration.

Parsing

struct s { int a; };
typedef struct s x;

the RecordDecl is not marked and then the two declaration should not be
grouped.

Following your proposal instead we have no way to understand that the
RecordDecl in the former case should not be output (RecordDecl does not
have TypeLoc where to store this info).

Take also in account that the bit you propose to add to typeloc is
already deducible now using the location trick above.

Hello.

The attached patch adds a new Boolean flag to TagDecl nodes.

The new flag, named IsDefinedInDeclarator, tells whether or not the
TagDecl is a definition that was syntactically occurring (possibly via
macro expansions) in a declarator. Examples include tag types defined in
function return types, function parameter types, variable or field
declarations and type arguments of cast or sizeof expressions.

The new flag is meant to help clients such as code style checkers.

This feels like the wrong solution. I'd like to hear from others, but
the best approach seems to be to extend TagTypeLoc with a bit that
indicates whether this use of the tag type was a reference or a
definition. That way, a pretty-printer could know exactly where to
put the definition.

Our aim is exactly that and it works perfectly using the proposed patch:
the RecordDecl inserted in DeclContext parsing

typedef struct s { int a; } x;

is marked with IsDefinedInDeclarator bit and then can be excluded from
pretty printer output.

Ah, I understand the issue now. It's less about the problem of "does this typedef contain the definition of struct s?" and more about the problem of "was struct s defined by itself or as part of some other declaration?". My proposed solution didn't cover the latter at all.

Next, analyzing TypedefDecl it could be easily
verified that the location of TypedefDecl is the same of RecordDecl and
then we can know easily that the record definition was embedded in the
typedef declaration.

Parsing

struct s { int a; };
typedef struct s x;

the RecordDecl is not marked and then the two declaration should not be
grouped.

Following your proposal instead we have no way to understand that the
RecordDecl in the former case should not be output (RecordDecl does not
have TypeLoc where to store this info).

Take also in account that the bit you propose to add to typeloc is
already deducible now using the location trick above.

Okay. Thank for you for the explanation. I've committed Enea's patch as r95586.

  - Doug

Douglas Gregor wrote:

[...]

Our aim is exactly that and it works perfectly using the proposed patch:
the RecordDecl inserted in DeclContext parsing

typedef struct s { int a; } x;

is marked with IsDefinedInDeclarator bit and then can be excluded from
pretty printer output.

Ah, I understand the issue now. It's less about the problem of "does
this typedef contain the definition of struct s?" and more about the
problem of "was struct s defined by itself or as part of some other
declaration?". My proposed solution didn't cover the latter at all.

Well, the solution we proposed worked _almost_ perfectly.

It turns out that tracking tag type *definitions* is not enough, since the declarator could also introduce a brand new tag type declaration which is not a definition.

The following example shows this:

TagDecl.patch (3.53 KB)

Douglas Gregor wrote:

[...]

Our aim is exactly that and it works perfectly using the proposed patch:
the RecordDecl inserted in DeclContext parsing

typedef struct s { int a; } x;

is marked with IsDefinedInDeclarator bit and then can be excluded from
pretty printer output.

Ah, I understand the issue now. It's less about the problem of "does
this typedef contain the definition of struct s?" and more about the
problem of "was struct s defined by itself or as part of some other
declaration?". My proposed solution didn't cover the latter at all.

Well, the solution we proposed worked _almost_ perfectly.

It turns out that tracking tag type *definitions* is not enough, since the declarator could also introduce a brand new tag type declaration which is not a definition.

The following example shows this:

struct Unknown* p;

This is pretty printed by clang as follows:

# clang -cc1 -ast-print test.c
[...snip...]
struct Unknown;
struct Unknown *p;

Right.

The fix is in the attached patch.

Looks good! Committed as r95991.

For clarity, we changed the same of the Boolean flag IsDefinedInDeclarator to IsEmbeddedInDeclarator:
its value is now computed as follows, exploiting
clang notion of "canonical" declaration:

Owned->setEmbeddedInDeclarator(Owned->isDefinition() ||
                                Owned->isCanonicalDecl());

I like IsEmbeddedInDeclarator much better! It describes what is happening very clearly.

  - Doug