[llvm] r319086 - Rename MCTargetOptionsCommandFlags.h to .def as it is not a normal/modular header as much as it is for stamping out some global/static variables
David Blaikie via llvm-commits
llvm-commits at lists.llvm.org
Thu Nov 30 15:03:48 PST 2017
On Thu, Nov 30, 2017 at 2:43 PM Reid Kleckner <rnk at google.com> wrote:
> I think .inc would be better, and there is prior art for it in
> Support/Unix & Windows.
>
Sure thing
> I feel like .def is supposed to be for a pile of macro (or something else)
> definitions.
>
Fair point, indeed - I was just deep in a bunch of build files that mostly
wanted "*.def" as textual headers anyway, so wasn't thinking of the broader
context.
> Unfortunately, .inc doesn't solve any of the problems that I actually have
> with this:
> - ctags doesn't find definitions in either file extension
> - syntax highlighting doesn't turn on by default for either extension
>
Isn't that what the "-*- C++ -*-" in the header comment is meant to
address? (it seems it doesn't, but perhaps I'm misunderstanding what it's
meant to help/not help)
> If a file has real source code in it, I'd really rather give it a source
> language suffix that most editors recognize. For example, we recently
> renamed the lit.cfg files to lit.cfg.py for this reason. This doesn't
> seem like a reasonable cost to pay for modularization.
>
Seems like a good thing to me to separate modular headers from non-modular
textual inclusions by something simple like a file extension. Non-modular
textual inclusions should be rare so it doesn't seem like a high cost to me
if they lose some editor integration.
- Dave
>
> On Thu, Nov 30, 2017 at 2:33 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>> It's not a header file though, either, right? It can't be/isn't intended
>> to be included in multiple files in the same program? It's code to be
>> stamped out, rather than a header, as such.
>>
>> My understanding was ".def" (or perhaps ".inc" would be a more suitable
>> extension?) files were for that purpose, so I was/am working on migrating
>> cases where we use a header in a way that's not really header-y to a
>> distinct extension to make that explicit (& simplify module config files -
>> so there's not a separate list of non-modular headers, but they can be more
>> simply identified by file extension - also useful for developers to realize
>> what they're interacting with).
>>
>> - Dave
>>
>> On Thu, Nov 30, 2017 at 2:29 PM Reid Kleckner <rnk at google.com> wrote:
>>
>>> This is... pretty objectionable, IMO. This file is not a .def file. It
>>> is not a pile of macros. It is C++ code. I really don't like this change.
>>> Surely we have a better way to mark headers as non-modular.
>>>
>>> On Mon, Nov 27, 2017 at 11:55 AM, David Blaikie via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> Author: dblaikie
>>>> Date: Mon Nov 27 11:55:16 2017
>>>> New Revision: 319086
>>>>
>>>> URL: http://llvm.org/viewvc/llvm-project?rev=319086&view=rev
>>>> Log:
>>>> Rename MCTargetOptionsCommandFlags.h to .def as it is not a
>>>> normal/modular header as much as it is for stamping out some global/static
>>>> variables
>>>>
>>>> Added:
>>>> llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.def
>>>> - copied, changed from r319085,
>>>> llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.h
>>>> Removed:
>>>> llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.h
>>>> Modified:
>>>> llvm/trunk/include/llvm/CodeGen/CommandFlags.def
>>>> llvm/trunk/tools/dsymutil/DwarfLinker.cpp
>>>> llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp
>>>> llvm/trunk/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
>>>> llvm/trunk/tools/llvm-mc/llvm-mc.cpp
>>>> llvm/trunk/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
>>>>
>>>> Modified: llvm/trunk/include/llvm/CodeGen/CommandFlags.def
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/CommandFlags.def?rev=319086&r1=319085&r2=319086&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/CodeGen/CommandFlags.def (original)
>>>> +++ llvm/trunk/include/llvm/CodeGen/CommandFlags.def Mon Nov 27
>>>> 11:55:16 2017
>>>> @@ -17,7 +17,7 @@
>>>> #include "llvm/IR/Instructions.h"
>>>> #include "llvm/IR/Intrinsics.h"
>>>> #include "llvm/IR/Module.h"
>>>> -#include "llvm/MC/MCTargetOptionsCommandFlags.h"
>>>> +#include "llvm/MC/MCTargetOptionsCommandFlags.def"
>>>> #include "llvm/MC/SubtargetFeature.h"
>>>> #include "llvm/Support/CodeGen.h"
>>>> #include "llvm/Support/CommandLine.h"
>>>>
>>>> Copied: llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.def
>>>> (from r319085, llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.h)
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.def?p2=llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.def&p1=llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.h&r1=319085&r2=319086&rev=319086&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.h (original)
>>>> +++ llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.def Mon Nov
>>>> 27 11:55:16 2017
>>>> @@ -19,7 +19,7 @@
>>>> #include "llvm/Support/CommandLine.h"
>>>> using namespace llvm;
>>>>
>>>> -cl::opt<MCTargetOptions::AsmInstrumentation> AsmInstrumentation(
>>>> +static cl::opt<MCTargetOptions::AsmInstrumentation> AsmInstrumentation(
>>>> "asm-instrumentation", cl::desc("Instrumentation of inline
>>>> assembly and "
>>>> "assembly source files"),
>>>> cl::init(MCTargetOptions::AsmInstrumentationNone),
>>>> @@ -28,40 +28,40 @@ cl::opt<MCTargetOptions::AsmInstrumentat
>>>> clEnumValN(MCTargetOptions::AsmInstrumentationAddress,
>>>> "address",
>>>> "instrument instructions with memory
>>>> arguments")));
>>>>
>>>> -cl::opt<bool> RelaxAll("mc-relax-all",
>>>> +static cl::opt<bool> RelaxAll("mc-relax-all",
>>>> cl::desc("When used with filetype=obj, "
>>>> "relax all fixups in the emitted
>>>> object file"));
>>>>
>>>> -cl::opt<bool> IncrementalLinkerCompatible(
>>>> +static cl::opt<bool> IncrementalLinkerCompatible(
>>>> "incremental-linker-compatible",
>>>> cl::desc(
>>>> "When used with filetype=obj, "
>>>> "emit an object file which can be used with an incremental
>>>> linker"));
>>>>
>>>> -cl::opt<bool> PIECopyRelocations("pie-copy-relocations", cl::desc("PIE
>>>> Copy Relocations"));
>>>> +static cl::opt<bool> PIECopyRelocations("pie-copy-relocations",
>>>> cl::desc("PIE Copy Relocations"));
>>>>
>>>> -cl::opt<int> DwarfVersion("dwarf-version", cl::desc("Dwarf version"),
>>>> +static cl::opt<int> DwarfVersion("dwarf-version", cl::desc("Dwarf
>>>> version"),
>>>> cl::init(0));
>>>>
>>>> -cl::opt<bool> ShowMCInst("asm-show-inst",
>>>> +static cl::opt<bool> ShowMCInst("asm-show-inst",
>>>> cl::desc("Emit internal instruction
>>>> representation to "
>>>> "assembly file"));
>>>>
>>>> -cl::opt<bool> FatalWarnings("fatal-warnings",
>>>> +static cl::opt<bool> FatalWarnings("fatal-warnings",
>>>> cl::desc("Treat warnings as errors"));
>>>>
>>>> -cl::opt<bool> NoWarn("no-warn", cl::desc("Suppress all warnings"));
>>>> -cl::alias NoWarnW("W", cl::desc("Alias for --no-warn"),
>>>> cl::aliasopt(NoWarn));
>>>> +static cl::opt<bool> NoWarn("no-warn", cl::desc("Suppress all
>>>> warnings"));
>>>> +static cl::alias NoWarnW("W", cl::desc("Alias for --no-warn"),
>>>> cl::aliasopt(NoWarn));
>>>>
>>>> -cl::opt<bool> NoDeprecatedWarn("no-deprecated-warn",
>>>> +static cl::opt<bool> NoDeprecatedWarn("no-deprecated-warn",
>>>> cl::desc("Suppress all deprecated
>>>> warnings"));
>>>>
>>>> -cl::opt<std::string>
>>>> +static cl::opt<std::string>
>>>> ABIName("target-abi", cl::Hidden,
>>>> cl::desc("The name of the ABI to be targeted from the
>>>> backend."),
>>>> cl::init(""));
>>>>
>>>> -static inline MCTargetOptions InitMCTargetOptionsFromFlags() {
>>>> +static MCTargetOptions InitMCTargetOptionsFromFlags() {
>>>> MCTargetOptions Options;
>>>> Options.SanitizeAddress =
>>>> (AsmInstrumentation ==
>>>> MCTargetOptions::AsmInstrumentationAddress);
>>>>
>>>> Removed: llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.h
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.h?rev=319085&view=auto
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.h (original)
>>>> +++ llvm/trunk/include/llvm/MC/MCTargetOptionsCommandFlags.h (removed)
>>>> @@ -1,80 +0,0 @@
>>>> -//===-- MCTargetOptionsCommandFlags.h --------------------------*- C++
>>>> -*-===//
>>>> -//
>>>> -// The LLVM Compiler Infrastructure
>>>> -//
>>>> -// This file is distributed under the University of Illinois Open
>>>> Source
>>>> -// License. See LICENSE.TXT for details.
>>>> -//
>>>>
>>>> -//===----------------------------------------------------------------------===//
>>>> -//
>>>> -// This file contains machine code-specific flags that are shared
>>>> between
>>>> -// different command line tools.
>>>> -//
>>>>
>>>> -//===----------------------------------------------------------------------===//
>>>> -
>>>> -#ifndef LLVM_MC_MCTARGETOPTIONSCOMMANDFLAGS_H
>>>> -#define LLVM_MC_MCTARGETOPTIONSCOMMANDFLAGS_H
>>>> -
>>>> -#include "llvm/MC/MCTargetOptions.h"
>>>> -#include "llvm/Support/CommandLine.h"
>>>> -using namespace llvm;
>>>> -
>>>> -cl::opt<MCTargetOptions::AsmInstrumentation> AsmInstrumentation(
>>>> - "asm-instrumentation", cl::desc("Instrumentation of inline
>>>> assembly and "
>>>> - "assembly source files"),
>>>> - cl::init(MCTargetOptions::AsmInstrumentationNone),
>>>> - cl::values(clEnumValN(MCTargetOptions::AsmInstrumentationNone,
>>>> "none",
>>>> - "no instrumentation at all"),
>>>> - clEnumValN(MCTargetOptions::AsmInstrumentationAddress,
>>>> "address",
>>>> - "instrument instructions with memory
>>>> arguments")));
>>>> -
>>>> -cl::opt<bool> RelaxAll("mc-relax-all",
>>>> - cl::desc("When used with filetype=obj, "
>>>> - "relax all fixups in the emitted
>>>> object file"));
>>>> -
>>>> -cl::opt<bool> IncrementalLinkerCompatible(
>>>> - "incremental-linker-compatible",
>>>> - cl::desc(
>>>> - "When used with filetype=obj, "
>>>> - "emit an object file which can be used with an incremental
>>>> linker"));
>>>> -
>>>> -cl::opt<bool> PIECopyRelocations("pie-copy-relocations", cl::desc("PIE
>>>> Copy Relocations"));
>>>> -
>>>> -cl::opt<int> DwarfVersion("dwarf-version", cl::desc("Dwarf version"),
>>>> - cl::init(0));
>>>> -
>>>> -cl::opt<bool> ShowMCInst("asm-show-inst",
>>>> - cl::desc("Emit internal instruction
>>>> representation to "
>>>> - "assembly file"));
>>>> -
>>>> -cl::opt<bool> FatalWarnings("fatal-warnings",
>>>> - cl::desc("Treat warnings as errors"));
>>>> -
>>>> -cl::opt<bool> NoWarn("no-warn", cl::desc("Suppress all warnings"));
>>>> -cl::alias NoWarnW("W", cl::desc("Alias for --no-warn"),
>>>> cl::aliasopt(NoWarn));
>>>> -
>>>> -cl::opt<bool> NoDeprecatedWarn("no-deprecated-warn",
>>>> - cl::desc("Suppress all deprecated
>>>> warnings"));
>>>> -
>>>> -cl::opt<std::string>
>>>> -ABIName("target-abi", cl::Hidden,
>>>> - cl::desc("The name of the ABI to be targeted from the
>>>> backend."),
>>>> - cl::init(""));
>>>> -
>>>> -static inline MCTargetOptions InitMCTargetOptionsFromFlags() {
>>>> - MCTargetOptions Options;
>>>> - Options.SanitizeAddress =
>>>> - (AsmInstrumentation ==
>>>> MCTargetOptions::AsmInstrumentationAddress);
>>>> - Options.MCRelaxAll = RelaxAll;
>>>> - Options.MCIncrementalLinkerCompatible = IncrementalLinkerCompatible;
>>>> - Options.MCPIECopyRelocations = PIECopyRelocations;
>>>> - Options.DwarfVersion = DwarfVersion;
>>>> - Options.ShowMCInst = ShowMCInst;
>>>> - Options.ABIName = ABIName;
>>>> - Options.MCFatalWarnings = FatalWarnings;
>>>> - Options.MCNoWarn = NoWarn;
>>>> - Options.MCNoDeprecatedWarn = NoDeprecatedWarn;
>>>> - return Options;
>>>> -}
>>>> -
>>>> -#endif
>>>>
>>>> Modified: llvm/trunk/tools/dsymutil/DwarfLinker.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.cpp?rev=319086&r1=319085&r2=319086&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/tools/dsymutil/DwarfLinker.cpp (original)
>>>> +++ llvm/trunk/tools/dsymutil/DwarfLinker.cpp Mon Nov 27 11:55:16 2017
>>>> @@ -55,7 +55,7 @@
>>>> #include "llvm/MC/MCStreamer.h"
>>>> #include "llvm/MC/MCSubtargetInfo.h"
>>>> #include "llvm/MC/MCTargetOptions.h"
>>>> -#include "llvm/MC/MCTargetOptionsCommandFlags.h"
>>>> +#include "llvm/MC/MCTargetOptionsCommandFlags.def"
>>>> #include "llvm/Object/MachO.h"
>>>> #include "llvm/Object/ObjectFile.h"
>>>> #include "llvm/Object/SymbolicFile.h"
>>>>
>>>> Modified: llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp?rev=319086&r1=319085&r2=319086&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp (original)
>>>> +++ llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp Mon Nov 27 11:55:16 2017
>>>> @@ -29,7 +29,7 @@
>>>> #include "llvm/MC/MCRegisterInfo.h"
>>>> #include "llvm/MC/MCSectionELF.h"
>>>> #include "llvm/MC/MCStreamer.h"
>>>> -#include "llvm/MC/MCTargetOptionsCommandFlags.h"
>>>> +#include "llvm/MC/MCTargetOptionsCommandFlags.def"
>>>> #include "llvm/Object/Decompressor.h"
>>>> #include "llvm/Object/ObjectFile.h"
>>>> #include "llvm/Support/Compression.h"
>>>>
>>>> Modified:
>>>> llvm/trunk/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp?rev=319086&r1=319085&r2=319086&view=diff
>>>>
>>>> ==============================================================================
>>>> ---
>>>> llvm/trunk/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp
>>>> (original)
>>>> +++
>>>> llvm/trunk/tools/llvm-mc-assemble-fuzzer/llvm-mc-assemble-fuzzer.cpp Mon
>>>> Nov 27 11:55:16 2017
>>>> @@ -24,7 +24,7 @@
>>>> #include "llvm/MC/MCSectionMachO.h"
>>>> #include "llvm/MC/MCStreamer.h"
>>>> #include "llvm/MC/MCSubtargetInfo.h"
>>>> -#include "llvm/MC/MCTargetOptionsCommandFlags.h"
>>>> +#include "llvm/MC/MCTargetOptionsCommandFlags.def"
>>>> #include "llvm/Support/MemoryBuffer.h"
>>>> #include "llvm/Support/CommandLine.h"
>>>> #include "llvm/Support/FileUtilities.h"
>>>>
>>>> Modified: llvm/trunk/tools/llvm-mc/llvm-mc.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-mc/llvm-mc.cpp?rev=319086&r1=319085&r2=319086&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/tools/llvm-mc/llvm-mc.cpp (original)
>>>> +++ llvm/trunk/tools/llvm-mc/llvm-mc.cpp Mon Nov 27 11:55:16 2017
>>>> @@ -26,7 +26,7 @@
>>>> #include "llvm/MC/MCSectionMachO.h"
>>>> #include "llvm/MC/MCStreamer.h"
>>>> #include "llvm/MC/MCSubtargetInfo.h"
>>>> -#include "llvm/MC/MCTargetOptionsCommandFlags.h"
>>>> +#include "llvm/MC/MCTargetOptionsCommandFlags.def"
>>>> #include "llvm/Support/CommandLine.h"
>>>> #include "llvm/Support/Compression.h"
>>>> #include "llvm/Support/FileUtilities.h"
>>>>
>>>> Modified: llvm/trunk/unittests/DebugInfo/DWARF/DwarfGenerator.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/DebugInfo/DWARF/DwarfGenerator.cpp?rev=319086&r1=319085&r2=319086&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/unittests/DebugInfo/DWARF/DwarfGenerator.cpp (original)
>>>> +++ llvm/trunk/unittests/DebugInfo/DWARF/DwarfGenerator.cpp Mon Nov 27
>>>> 11:55:16 2017
>>>> @@ -27,7 +27,7 @@
>>>> #include "llvm/MC/MCRegisterInfo.h"
>>>> #include "llvm/MC/MCStreamer.h"
>>>> #include "llvm/MC/MCSubtargetInfo.h"
>>>> -#include "llvm/MC/MCTargetOptionsCommandFlags.h"
>>>> +#include "llvm/MC/MCTargetOptionsCommandFlags.def"
>>>> #include "llvm/PassAnalysisSupport.h"
>>>> #include "llvm/Support/LEB128.h"
>>>> #include "llvm/Support/TargetRegistry.h"
>>>> @@ -155,8 +155,8 @@ llvm::Error dwarfgen::Generator::init(Tr
>>>> MC.reset(new MCContext(MAI.get(), MRI.get(), MOFI.get()));
>>>> MOFI->InitMCObjectFileInfo(TheTriple, /*PIC*/ false, *MC);
>>>>
>>>> - MCTargetOptions Options;
>>>> - MAB = TheTarget->createMCAsmBackend(*MRI, TripleName, "", Options);
>>>> + MCTargetOptions MCOptions = InitMCTargetOptionsFromFlags();
>>>> + MAB = TheTarget->createMCAsmBackend(*MRI, TripleName, "", MCOptions);
>>>> if (!MAB)
>>>> return make_error<StringError>("no asm backend for target " +
>>>> TripleName,
>>>> inconvertibleErrorCode());
>>>> @@ -179,7 +179,6 @@ llvm::Error dwarfgen::Generator::init(Tr
>>>>
>>>> Stream = make_unique<raw_svector_ostream>(FileBytes);
>>>>
>>>> - MCTargetOptions MCOptions = InitMCTargetOptionsFromFlags();
>>>> MS = TheTarget->createMCObjectStreamer(
>>>> TheTriple, *MC, std::unique_ptr<MCAsmBackend>(MAB), *Stream,
>>>> std::unique_ptr<MCCodeEmitter>(MCE), *MSTI, MCOptions.MCRelaxAll,
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171130/eb49b044/attachment.html>
More information about the llvm-commits
mailing list