Should isSimpleTypeSpecifier return true for _Bool?

carlos4242 · November 13, 2023, 12:06pm

Hi, this is a fairly simple query I think? Bourne of some client code in the “Clang importer” part of the Swift compiler. That importer reads the above function to decide how to interpret macros.

I noticed that it was not treating _Bool as a simple type, but from C99 onward, it is, right?

By comparison tok::kw_bool returns true.

To patch my code, I added tok::kw__Bool locally in my source code copy into the same part of the case statement so that it returns true.

Is that right?? Am I missing some subtleties? Should I submit a suggested patch?

Cheers,
Carl

AaronBallman · November 13, 2023, 12:49pm

I think this is a known deficiency; at least, we’ve got a FIXME comment about it in the source code:

github.com

llvm/llvm-project/blob/68ea8a674c76d25e5e52717aa8f78fb25679fdb0/clang/lib/Sema/SemaDecl.cpp#L131


      
            bool WantClassName;
            bool AllowTemplates;
            bool AllowNonTemplates;
          };
          
          } // end anonymous namespace
          
          /// Determine whether the token kind starts a simple-type-specifier.
          bool Sema::isSimpleTypeSpecifier(tok::TokenKind Kind) const {
            switch (Kind) {
            // FIXME: Take into account the current language when deciding whether a
            // token kind is a valid type specifier
            case tok::kw_short:
            case tok::kw_long:
            case tok::kw___int64:
            case tok::kw___int128:
            case tok::kw_signed:
            case tok::kw_unsigned:
            case tok::kw_void:
            case tok::kw_char:
            case tok::kw_int:

We can call getLangOpts() to get a more accurate answer (we do that for some of the other tokens already).

carlos4242 · November 13, 2023, 3:17pm

That makes perfect sense. bool is … only native to C++ too? I might be wrong on that and I definitely wouldn’t want to get into an argument on it.

I’ll submit a PR then. Or at least attempt one. It looks like it’s all on GitHub now, instead of Phabricator, which makes it easier for me to understand!

Thank you!

AaronBallman · November 13, 2023, 4:11pm

bool as a keyword exists in all versions of C++ and in C23 and later. _Bool exists as a keyword in all C language modes (standard feature of C99 and an extension in C89, not exposed in C++).

carlos4242 · November 14, 2023, 4:08am

PR raised: [clang] Sema::isSimpleTypeSpecifier return true for _Bool in c99 by carlos4242 · Pull Request #72204 · llvm/llvm-project · GitHub

I’m unsure how I might unit test this, or if that’s not appropriate in this case?

AaronBallman · November 14, 2023, 12:38pm

Thank you for the PR! I’ll think about the testing question as I review the patch; hopefully we can devise a way to test these changes specifically.