[clang-tools-extra] r341459 - [clangd] Avoid enum class+enumValN to avoid GCC bug(?), and use consistent style.

[Sam McCall via cfe-commits]

It turned out to be a different bug: the problem was referring to
`Format::YAML` in the initializer for a variable also named `Format`.
This is legal but old versions of GCC get this wrong.
As usual with buildbot failures, I was throwing things at the wall to see
what sticks.

Regarding style - either would work, there were 2x enum and 2x enum class.
My reasons for leaning towards enum here is that for this command-line flag
pattern
 - it mainly leads to repeating the type name in a context where the type
is obvious
 - there's minimal risk/consequence to a namespace conflict as we're in an
anonymous namespace in a CC file
 - there's often not a second good name (need one for the flag + one for
the enum), so it's a bad name that ends up repeated
But if you feel strongly about it, feel free to flip it.

On Thu, Sep 6, 2018 at 11:45 AM Ilya Biryukov <ibiryukov at google.com> wrote:

> Any pointers to the GCC bug/breakage mentioned?
>
> On Thu, Sep 6, 2018 at 11:44 AM Ilya Biryukov <ibiryukov at google.com>
> wrote:
>
>> +1 for consistent style, but why not use enum class everywhere instead?
>>
>> On Wed, Sep 5, 2018 at 12:41 PM Sam McCall via cfe-commits <
>> cfe-commits at lists.llvm.org> wrote:
>>
>>> Author: sammccall
>>> Date: Wed Sep  5 03:39:58 2018
>>> New Revision: 341459
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=341459&view=rev
>>> Log:
>>> [clangd] Avoid enum class+enumValN to avoid GCC bug(?), and use
>>> consistent style.
>>>
>>> Modified:
>>>
>>> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
>>>     clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
>>>
>>> Modified:
>>> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=341459&r1=341458&r2=341459&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
>>> (original)
>>> +++
>>> clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
>>> Wed Sep  5 03:39:58 2018
>>> @@ -60,7 +60,7 @@ static llvm::cl::opt<bool> MergeOnTheFly
>>>          "MapReduce."),
>>>      llvm::cl::init(true), llvm::cl::Hidden);
>>>
>>> -enum class Format { YAML, Binary };
>>> +enum Format { YAML, Binary };
>>>  static llvm::cl::opt<Format>
>>>      Format("format", llvm::cl::desc("Format of the index to be
>>> written"),
>>>             llvm::cl::values(
>>>
>>> Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=341459&r1=341458&r2=341459&view=diff
>>>
>>> ==============================================================================
>>> --- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
>>> +++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Wed Sep  5
>>> 03:39:58 2018
>>> @@ -36,12 +36,6 @@ static llvm::cl::opt<bool>
>>>             llvm::cl::desc("Use experimental Dex static index."),
>>>             llvm::cl::init(true), llvm::cl::Hidden);
>>>
>>> -namespace {
>>> -
>>> -enum class PCHStorageFlag { Disk, Memory };
>>> -
>>> -} // namespace
>>> -
>>>  static llvm::cl::opt<Path> CompileCommandsDir(
>>>      "compile-commands-dir",
>>>      llvm::cl::desc("Specify a path to look for compile_commands.json.
>>> If path "
>>> @@ -54,10 +48,7 @@ static llvm::cl::opt<unsigned>
>>>                         llvm::cl::init(getDefaultAsyncThreadsCount()));
>>>
>>>  // FIXME: also support "plain" style where signatures are always
>>> omitted.
>>> -enum CompletionStyleFlag {
>>> -  Detailed,
>>> -  Bundled,
>>> -};
>>> +enum CompletionStyleFlag { Detailed, Bundled };
>>>  static llvm::cl::opt<CompletionStyleFlag> CompletionStyle(
>>>      "completion-style",
>>>      llvm::cl::desc("Granularity of code completion suggestions"),
>>> @@ -106,6 +97,7 @@ static llvm::cl::opt<bool> Test(
>>>          "Intended to simplify lit tests."),
>>>      llvm::cl::init(false), llvm::cl::Hidden);
>>>
>>> +enum PCHStorageFlag { Disk, Memory };
>>>  static llvm::cl::opt<PCHStorageFlag> PCHStorage(
>>>      "pch-storage",
>>>      llvm::cl::desc("Storing PCHs in memory increases memory usages, but
>>> may "
>>> @@ -167,7 +159,6 @@ static llvm::cl::opt<Path> YamlSymbolFil
>>>      llvm::cl::init(""), llvm::cl::Hidden);
>>>
>>>  enum CompileArgsFrom { LSPCompileArgs, FilesystemCompileArgs };
>>> -
>>>  static llvm::cl::opt<CompileArgsFrom> CompileArgsFrom(
>>>      "compile_args_from", llvm::cl::desc("The source of compile
>>> commands"),
>>>      llvm::cl::values(clEnumValN(LSPCompileArgs, "lsp",
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>
>>
>> --
>> Regards,
>> Ilya Biryukov
>>
>
>
> --
> Regards,
> Ilya Biryukov
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20180906/e994c8c8/attachment-0001.html>