[cfe-dev] Cleanup on aisle DebugInfo enums

[David Blaikie via cfe-dev]

On Fri, Feb 5, 2016 at 11:30 AM, Robinson, Paul <
Paul_Robinson at playstation.sony.com> wrote:

> what's the motivation not to use the LLVM versions of these enums
>
>
>
> I originally implemented the Clang bits for debugger tuning using the LLVM
> enum (r256063).  However this caused build failures in clang-tools-extra
> (which I didn't have checked out locally) because having CodeGenOptions.h
> pull in the extra LLVM header caused confusion in some unqualified name
> references.
>
> Rather than introduce the extra qualifiers in clang-tools-extra (I looked,
> it would have taken a bit to sort everything out) I undid the dependency on
> the LLVM headers and just replicated the enum in Clang.  The only place in
> Clang that still "really" needed the LLVM enum is BackendUtil.cpp (natch)
> and it is careful not to assume the two enums are defined the same way.
>

Still seems a bit subtle - are there reasons we should avoid using the LLVM
enum apart from the unknown issue with clang-tools-extra? (if we generally
keep these types/enums separate, that's OK - but don't want to propagate a
workaround if we don't fully understand what we're working around)


> This got committed in r256078.
>
> However, I wasn't thorough enough about digging out the LLVM dependencies,
> and Driver is still using the LLVM enum.
>
> If I'd gotten the Driver part right the first time around, Benny would've
> had to move both enums in his patch.
>
>
>
> This Clang-versus-LLVM consideration applies only to the tuning enum, btw;
> LLVM doesn't have an analog of DebugInfoKind.
>
> --paulr
>
>
>
> *From:* David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* Friday, February 05, 2016 11:06 AM
> *To:* Robinson, Paul; Adrian Prantl; Eric Christopher; Benjamin Kramer
> *Cc:* cfe-dev at lists.llvm.org
> *Subject:* Re: [cfe-dev] Cleanup on aisle DebugInfo enums
>
>
>
> Sounds plausible.
>
> (just to check: what's the motivation not to use the LLVM versions of
> these enums if they match the desired semantics? (I imagine there are many
> good reasons not to use them, just figure it's worth writing down in this
> thread so it's clear for historic purposes))
>
>
>
> On Fri, Feb 5, 2016 at 10:57 AM, Robinson, Paul via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
> In r259489, Benny Kramer moved the DebugInfoKind enum into its own
> header, include/clang/Driver/DebugInfoKind.h. This was to prevent
> Driver from pulling in headers from Frontend.  However, this leaves
> the enum in kind of a weird place, I think Basic would be better (and
> not re-introduce the problem Benny was solving).
>
> Wondering why the tuning enum didn't cause the same problem, I found
> that when I created the Clang enum, I didn't make Driver use it; it's
> still using the LLVM version of the enum.  While this is not
> technically wrong (you could rearrange the values and it would do no
> harm) it's an inconsistency within Clang.
>
> But, if I make Driver use the tuning enum from Clang's CodeGenOptions,
> then we'll have the same problem that prompted r259489 in the first
> place--Driver depending on Frontend. Rather than create Yet Another
> Header, it would be convenient to put the tuning enum in the same
> place as DebugInfoKind, as they are topically related and commonly
> used in the same places.
>
> I propose to do the following:
>
> Patch #1 (totally mechanical)
> - move DebugInfoKind.h from Driver to Basic
> - rename it to DebugInfoOptions.h
> This will take care of all the #include renaming in one go.
>
> Patch #2 (basically mechanical)
> - move the DebuggerKind enum from CodeGenOptions.h to DebugInfoOptions.h
> - fix up Driver to use this enum instead of the LLVM equivalent.
>
> Or I can do it all at once, the combined patch would still be
> reasonable I think.
>
> Sound okay?
> --paulr
>
> _______________________________________________
> cfe-dev mailing list
> cfe-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20160205/429e802f/attachment.html>