[clang] initializeBuiltins is setting all `LangBuiltin` not defined in `builtinIsSupported` to true

[farzonl]

Recently when modifying (#101543) the names of a few HLSL builtins We noticed test failures on coroutine-builtins.cpp:18 when DERRORS was defined.

The expected behavior with that flag is that __builtin_coro_alloc(); would not be defined because without -std=c++20 the coroutine language ops are not set.

What is actually happening is that initializeBuiltins is iterating through all builtins and running builtinIsSupported which defaults to true.

In the below screenshot you can see two debug windows, one on main and one on the effected branch. On the main branch __builtin_coro_alloc evaluates to UnresolvedLookupExpr. On the effected branch its a DeclRef pointing at __builtin_hlsl_all

I ended up fixing this issue for now by adding HLSL as a language option: 0bb647d

That said a code restructure is needed or someone else working on LangBuiltin is going to run into this because this ends up being a bad pattern. Below I enumerate two reasons why:

1. if there are missing lang opts in this list then all those builtins get turned on by default without the maintainers of the languages options knowing. This should be an opt in for language options.
2. __builtin_coro_alloc should always be associated with COR_LANG whether or not LangOpts.Coroutines is true or false. That way if the builtin is seen and the feature is not enabled builtinIsSupported would return false.

So instead of

if (!LangOpts.Coroutines && (BuiltinInfo.Langs & COR_LANG))
    return false;
...
return true;

We could do

if (BuiltinInfo.Langs & COR_LANG) {
    if(!LangOpts.Coroutines) 
        return false;
    return true;
}
....
return false;

I'm open to other code chages here aswell this is just one suggestion on how to fix this issue.

@bogner who pair debugged this issue with me.

[llvmbot]

@llvm/issue-subscribers-coroutines

Author: Farzon Lotfi (farzonl)

Recently when modifying (#101543) the names of a few HLSL builtins We noticed test failures on [coroutine-builtins.cpp:18](https://github.com/llvm/llvm-project/blob/main/clang/test/SemaCXX/coroutine-builtins.cpp#L18) when `DERRORS` was defined.

The expected behavior with that flag is that __builtin_coro_alloc(); would not be defined because without -std=c++20 the coroutine language ops are not set.

What is actually happening is that initializeBuiltins is iterating through all builtins and running builtinIsSupported which defaults to true.

In the below screenshot you can see two debug windows, one on main and one on the effected branch. On the main branch __builtin_coro_alloc evaluates to UnresolvedLookupExpr. On the effected branch its a DeclRef pointing at __builtin_hlsl_all

I ended up fixing this issue for now by adding HLSL as a language option: 0bb647d

That said a code restructure is needed or someone else working on LangBuiltin is going to run into this because this ends up being a bad pattern. Below I enumerate two reasons why:

1. if there are missing lang opts in this list then all those builtins get turned on by default without the maintainers of the languages options knowing. This should be an opt in for language options.
2. __builtin_coro_alloc should always be associated with COR_LANG whether or not LangOpts.Coroutines is true or false. That way if the builtin is seen and the feature is not enabled builtinIsSupported would return false.

So instead of

if (!LangOpts.Coroutines && (BuiltinInfo.Langs & COR_LANG))
    return false;
...
return true;

We could do

if (BuiltinInfo.Langs & COR_LANG) {
    if(!LangOpts.Coroutines) 
        return false;
    return true;
}
....
return false;

I'm open to other code chages here aswell this is just one suggestion on how to fix this issue.

@bogner who pair debugged this issue with me.

[llvm-beanz]

tagging a few people for visibility: @Endilll, @cor3ntin, @erichkeane, @shafik, @apolloww, & @yuxuanchen1997,