If the compiler is doing something that isn't what the source says to do, the 
compiler should say so.  That's the principle behind this patch.

Even though the reason we ignore one attribute is because of a different 
attribute whose anticipated use-case is a temporary debugging measure, the 
principle should still apply.  Currently we're inconsistent, because we error 
if the attributes are on the same declaration but silently ignore one if 
they're on different declarations.  That inconsistency is another Bad Thing.
After this patch, we'd error on the same declaration and warn on different 
declarations, which is also inconsistent.  I was contemplating downgrading that 
error to a warning, so that the behavior is the same all around.
--paulr

From: Sean Silva [mailto:[email protected]]
Sent: Thursday, December 11, 2014 5:33 PM
To: Robinson, Paul
Cc: [email protected]; Aaron Ballman ([email protected])
Subject: Re: [PATCH] Diagnose 'optnone' versus conflicting attrs on another decl

Sort of a bigger-picture question: does it make sense to warn about this? I 
thought that optnone was just for debugging; is there a use case where the user 
would put optnone on something when debugging and actually care about whether 
optnone is overriding something else?

-- Sean Silva

On Thu, Dec 11, 2014 at 4:27 PM, Robinson, Paul 
<[email protected]<mailto:[email protected]>> 
wrote:
Diagnose when attribute 'optnone' conflicts with attributes on a
different declaration.

I modeled this on how dllimport/dllexport are handled; specifically,
I made 'optnone' continue to "win" over 'always_inline' and 'minsize'.
This preserves the previous "winner" behavior but adds the diagnostics
that were requested.

I have two questions about all this.

First, the diagnostics point to the attribute being ignored, but not
to the attribute that caused it to be ignored.  Is that okay?  This
would be a problem for dllimport/dllexport as well as the new cases.

Second, now that there are 5 similar cases (dllimport v. dllexport,
plus optnone v. always_inline/minsize) it seems like there could be
an opportunity to refactor some of that diagnostic checking into a
templated helper function, along the lines of mergeVisibilityAttr.
Should I pursue that? (Not clear it's much of a win... but maybe.)

Thanks,
--paulr


_______________________________________________
cfe-commits mailing list
[email protected]<mailto:[email protected]>
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

Reply via email to