[clang] Include clang config.h in LangStandards.cpp

[porglezomp]

Summary

This is necessary in order to pick up the default C/C++ standard from
the CLANG_DEFAULT_STD_C(XX) defines. This fixes a bug that was
introduced when this default language standard code was moved from
Frontend to Basic, making compilers ignore the configured default
language version override.

Fixes a bug introduced by D121375.

[hokein]

Thanks!

[dexonsmith]

Can we add a test for this?

[sammccall]

In D124974#3494279, @dexonsmith wrote:

Can we add a test for this?

I think the problem with testing it is that this variable comes from the CMake config, and in the default configuration (as used by all the bots) it's not set.

[porglezomp]

I would be interested in adding some kind of test for this, but am not sure how.

I think the robust general-purpose check here would be to ensure that no file is referring to symbols defined by the various config.h etc. files without including those files, but am not sure how to do that analysis, or if it's feasible with legacy code. That's obviously a much larger thing to test than this one change, and might run into problems if there are any define flags that don't come from the CMake config.

[porglezomp]

Unless anyone has an idea about how to test this, I'm going to commit this without adding a test.

[dexonsmith]

In D124974#3494374, @sammccall wrote:

In D124974#3494279, @dexonsmith wrote:

Can we add a test for this?

I think the problem with testing it is that this variable comes from the CMake config, and in the default configuration (as used by all the bots) it's not set.

Maybe you could use the same CMake config to set a lit variable, and use that lit variable to test that we get the configured default language variant.

[porglezomp]

Ah, so it'd be a test that passes pretty trivially on any bot that doesn't have a custom version of CLANG_DEFAULT_STD_C and CLANG_DEFAULT_STD_CXX like the default config, but could get caught by any other bots that do set it?

What's the best way to get the compiler to report the standard it's using for this test?

For C++ I can do:

clang++ -xc++ -dM -E - < /dev/null | grep cplusplus

And it will print e.g.

#define __cplusplus 199711L

But that requires quite some decoding to compare against CLANG_DEFAULT_STD_CXX which would look like LangStandard::lang_gnucxx98.

[dexonsmith]

In D124974#3504986, @porglezomp wrote:

Ah, so it'd be a test that passes pretty trivially on any bot that doesn't have a custom version of CLANG_DEFAULT_STD_C and CLANG_DEFAULT_STD_CXX like the default config, but could get caught by any other bots that do set it?

What's the best way to get the compiler to report the standard it's using for this test?

For C++ I can do:

clang++ -xc++ -dM -E - < /dev/null | grep cplusplus

And it will print e.g.

#define __cplusplus 199711L

But that requires quite some decoding to compare against CLANG_DEFAULT_STD_CXX which would look like LangStandard::lang_gnucxx98.

Yeah, that seems awkward.

My concern with lacking a test is that someone could easily remove this in the future when trying to prune unused #includes.

Another solution might be to change this to a compile error. I think it'd work to change the #cmakedefine logic to:

#cmakedefine CLANG_DEFAULT_STD_C LangStandard::lang_${CLANG_DEFAULT_STD_C}
// Always #define something so that missing the config.h #include at use sites
// becomes a compile error.
#ifndef CLANG_DEFAULT_STD_C
#define CLANG_DEFAULT_STD_C LangStandard::lang_unknown
#endif

and then update the code to:

case Language::C:
  if (CLANG_DEFAULT_STD_C != LangStandard::lang_unknown)
    return CLANG_DEFAULT_STD_C;

  // The PS4 uses C99 as the default C standard.
  if (T.isPS4())
    return LangStandard::lang_gnu99;
  return LangStandard::lang_gnu17;

(without the #ifdef)

WDYT?

[porglezomp]

That looks good to me. I'm testing it out locally to make sure it still works right and also catches the issue, and will plan put up an updated patch in the morning.

[porglezomp]

Add changes suggested by Duncan to make CLANG_DEFAULT_STD_C more misuse-resistant

[porglezomp]

Arcanist accidentally threw away the first commit in the sequence...

[porglezomp]

I haven't used arc in a little while and am having trouble sorry for the emails

[dexonsmith]

LGTM, thanks! One comment online below.

clang/include/clang/Config/config.h.cmake

19 ↗

(On Diff #429275)

I just noticed that the other comments in this file are C-style. Probably the new ones should be too.

clang/lib/Basic/LangStandards.cpp

 //===--- LangStandards.cpp - Language Standard Definitions ----------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Basic/LangStandard.h"
+#include "clang/Config/config.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/ErrorHandling.h"
 using namespace clang;
 
 #define LANGSTANDARD(id, name, lang, desc, features)                           \
   static const LangStandard Lang_##id = {name, desc, features, Language::lang};
 #include "clang/Basic/LangStandards.def"