<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">Am 13.01.14 03:43, schrieb Aaron
      Ballman:<br>
    </div>
    <blockquote
cite="mid:CAAt6xTu+9jvhRbpYy=Kgh=gKWYkeSjYxKZM+ML79_k40GF8-wA@mail.gmail.com"
      type="cite">
      <pre wrap="">New patch attached with some suggestions implemented. Changes include:

1) Updated the cmake file because the name of the tablegen file was
updated (this should have been in the initial patch)
2) Lexing unknown syntax now attempts better recovery. This allows us
to be a bit more future-proof
3) Unknown syntax is now handled as a warning instead of an error.
However, malformed __has_attribute is still treated as an error (for
instance, __has_attribute[haha] will be an error, but
__has_attribute([haha]) will be a warning). The warning may warrant
its own diagnostic group instead of piggy-backing on UnknownAttribute.
4) C++11 language option checking was moved into tablegen instead of
the preprocessor
5) Updated test cases for the new warning, future-proofing and the
movement of C++11 language option checking

What this patch does not address is:

* It's still using __has_attribute while we discuss an improved name
* The default __has_attribute(ident) functionality still checks for
all attribute names (changing this would be a separate patch,
regardless)

One other thing to consider with regards to naming -- another approach
we could take would be to not encode the syntax directly in the
feature macro, but instead use separate macros (with its analogous
currently-proposed syntax in comments):

__has_gnu_attribute(ident)  // __has_attribute(__attribute__((ident)))
__has_declspec_attribute(ident)  // __has_attribute(__declspec(ident))
__has_generalized_attribute(ident) // __has_attribute([[ident]])
__has_keyword_attribute(ident)  // __has_attribute(ident)</pre>
    </blockquote>
    <br>
    What about the following instead (could also swap the order of these
    arguments):<br>
    <br>
    <pre wrap="">__has_attribute(gnu,         ident) // __has_attribute(__attribute__((ident)))
__has_attribute(declspec,    ident) // __has_attribute(__declspec(ident))
__has_attribute(generalized, ident) // __has_attribute([[ident]])
__has_attribute(keyword,     ident) // __has_attribute(ident)</pre>
    <br>
    <blockquote
cite="mid:CAAt6xTu+9jvhRbpYy=Kgh=gKWYkeSjYxKZM+ML79_k40GF8-wA@mail.gmail.com"
      type="cite">
      <pre wrap="">

This would still be future-proof regardless of new attribute syntaxes,
and would solve the naming question. The downside is that I think it's
less discoverable, and means we're using N identifiers instead of one
identifier for the feature test. But I figured it'd be best to bring
it up as an option in case it sways anyone's opinions.

~Aaron


On Sun, Jan 12, 2014 at 8:40 PM, Alp Toker <a class="moz-txt-link-rfc2396E" href="mailto:alp@nuanti.com"><alp@nuanti.com></a> wrote:
</pre>
      <blockquote type="cite">
        <pre wrap="">
On 12/01/2014 23:57, Aaron Ballman wrote:
</pre>
        <blockquote type="cite">
          <pre wrap="">
Also, as I mentioned above, the same argument could be made for any of
the feature test macros. Saying "but there's documentation" is not a
compelling argument to me.
</pre>
        </blockquote>
        <pre wrap="">

Hi David,

The point Aaron makes here is significant.

The reality goes even further: feature detection macros are little more than
"fun" opportunistic features intended to save the hassle of a configure-time
feature check. Many of your criticisms are valid and apply to other
detection macros but that doesn't stop them being useful in a lot of trivial
use cases.

Serious projects* with an expectation of full-featured portability and
guarantees that a given language construct works as intended shouldn't be
using these macros in the first place.

So I'd suggest getting the recommended usage documented as a resolution.

* LLVM's Compiler.h is an exception because it's biased and optimizes for
bootstrap clang builds. Much of Compiler.h was simply disabled in non-clang
builds until very recently without anyone noticing.

Alp.


--
<a class="moz-txt-link-freetext" href="http://www.nuanti.com">http://www.nuanti.com</a>
the browser experts

</pre>
        <br>
        <fieldset class="mimeAttachmentHeader"></fieldset>
        <br>
        <pre wrap="">_______________________________________________
cfe-dev mailing list
<a class="moz-txt-link-abbreviated" href="mailto:cfe-dev@cs.uiuc.edu">cfe-dev@cs.uiuc.edu</a>
<a class="moz-txt-link-freetext" href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a>
</pre>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>