Parser TODO/FIXME

cfe-dev,
So starting with a few of the FIXME/TODO entries found in Parser.cpp listed by line number.

230: TODO: Move this someplace more useful.
    Is that more useful place Builtin::Context::InitializeBuiltins?
239: TODO: add a 'TST_builtin' type
    Would this just include adding it to the TST enumeration and DeclSpec::getSpecifierName? I don't think this has to be added to ParseDeclarationSpecifiers, or ConvertDeclSpecToType because those would not parse a builtin.
305: TODO: Invoke action for top-level semicolon
    Is this always a null action?
309-311 FIXME: Disable extensions warnings.
    Is this as simple as Diags.setWarnOnExtension(false/true);
318: TODO: Invoke action for top-level asm.
    What action should this be? I am not sure how an __asm("...")__ at file scope is handled
355-359: TODO:
    Which of these productions are not handled? It seems as though the first two are handled, with the [!C99] init-declarator-list ';' not being handled. I may be reading it wrong.

Patrick

cfe-dev,
So starting with a few of the FIXME/TODO entries found in Parser.cpp listed by line number.

Great!

230: TODO: Move this someplace more useful.
    Is that more useful place Builtin::Context::InitializeBuiltins?

That probably won't work because it doesn't have ASTContext information. The tricky thing here is that this needs to be registered in the symbol table for any implementation of the Actions interface (in order to correctly parse the file). As such, it really should be in the Parser. I'd just leave it where it is for now until we grow other builtin types.

239: TODO: add a 'TST_builtin' type
    Would this just include adding it to the TST enumeration and DeclSpec::getSpecifierName? I don't think this has to be added to ParseDeclarationSpecifiers, or ConvertDeclSpecToType because those would not parse a builtin.

There are two possible resolutions to this. The TODO should probably be resolved in favor of asking the target what the right type for __builtin_va_list is. Once we had that, we could just make it a typedef for the correct type without having to add a new TST enum entry.

305: TODO: Invoke action for top-level semicolon
    Is this always a null action?

Yep, it would just be a new 'void Action::ParseTopLevelSemi(SourceLocation SemiLoc) {}' method. No current clients would implement it.

309-311 FIXME: Disable extensions warnings.
    Is this as simple as Diags.setWarnOnExtension(false/true);

Yes exactly. It should save the current state, then set it to false, then restore the previous state on return.

318: TODO: Invoke action for top-level asm.
    What action should this be? I am not sure how an __asm("...")__ at file scope is handled

I wouldn't worry about this until asm parsing is more refined.

355-359: TODO:
    Which of these productions are not handled? It seems as though the first two are handled, with the [!C99] init-declarator-list ';' not being handled. I may be reading it wrong.

You're right, we're farther along here. I updated the todo markers. One remaining issue is that, in C99 mode,
we don't emit an extension diagnostic for C files like:

y;

which is "implicit int y;", which should be rejected in C99 mode but we accept as an extension.

-Chris