On Thu, Nov 21, 2019 at 14:09:41 +0100, Ard Biesheuvel wrote:
> > > I don't like this at all.
> > >
> > > If two expressions happen to evaluate to the same constant, we should
> > > be able to compare them using C code.
> > >
> > > I think we should disable the warning instead.
> >
> > I agree with that as a general rule, and agree that disabling the
> > warning may be preferable in other contexts.
> >
> > But I somewhat disagree with referring to *this instance* as two
> > expressions evaluating to the same constant. This is entirely a
> > preprocessor event, somewhat obscured by the Pcd syntax.
> > The test is "has the default baudrate been requested or has a specific
> > one been requested?".
>
> That may be true, but moving from C conditionals to CPP conditionals
> obscures the code, and reduces test coverage, so I think it should be
> avoided when possible.
I don't see what test coverage this would change. Even in the NOOPT
case, only the codegen would be affected - only one of the two cases
would be reachable. In all other cases, the compiler would go
if (115200 != 0) {
/* Oh, I'll just ignore any else clauses then ... */
I think the pre-existing code obscures what is actually going on and
that the change clarifies it (but could be prettified).
Now, of course, why we have a platform-specific (and platform-global)
override variable to make a library ignore its inputs is another
interesting question.
So. Rock-paper-scissors or coin-toss?
/
Leif
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#51068): https://edk2.groups.io/g/devel/message/51068
Mute This Topic: https://groups.io/mt/32999801/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
