From c588672b93662be28437026a784bb725b32dc320 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 20 Jan 2022 08:34:14 +0200 Subject: [PATCH] Add support for checks reuse Specifically: 1. A check's option name can now be marked as "unprefixable": `// !`. 2. A check can now list a number of base checks: `// : ...`. 3. Duplicate options are now automatically suppressed. --- README.md | 48 ++-- libbuild2-autoconf-tests/basics/testscript | 184 +++++++++++++-- .../libbuild2/autoconf/buildfile | 44 +++- .../libbuild2/autoconf/rule.cxx | 218 +++++++++++++++--- 4 files changed, 428 insertions(+), 66 deletions(-) diff --git a/README.md b/README.md index 56be3e5..a2f4e5b 100644 --- a/README.md +++ b/README.md @@ -14,8 +14,10 @@ program to check if the feature is present. Instead, they are set to static expected values based on the platform/compiler macro checks (see note at the beginning of [Project Configuration][proj-config] for rationale). -See [`libbuild2/autoconf/checks/`][checks] for the list of available build-in -checks. +See [`libbuild2/autoconf/checks/`][checks] for the list of available built-in +checks. Submit requests for new checks as issues. Submit implementations of +new checks (or any other improvements) as PRs or patches. + ## Using in your projects @@ -74,7 +76,7 @@ h{config}: in{config} } ``` -This mechanism can also be used to override the build-in checks, for example: +This mechanism can also be used to override the built-in checks, for example: ``` h{config}: in{config} @@ -83,7 +85,7 @@ h{config}: in{config} } ``` -The build-in checks can be prefixed in order to avoid clashes with similarly +The built-in checks can be prefixed in order to avoid clashes with similarly named macros in other headers. This is an especially good idea if the resulting header is public. To enable this, we specify the prefix with the `autoconf.prefix` variable and then use the prefixed versions of @@ -104,7 +106,7 @@ h{config}: in{config} ``` Note that `autoconf.prefix` only affects the lookup of the built-in checks. -Custom substitutions and overrides of build-in checks must include the +Custom substitutions and overrides of built-in checks must include the prefix. For example: ``` @@ -116,24 +118,37 @@ h{config}: in{config} } ``` +Note also that some built-in check names are *unprefixable*, usually because +they are standard macro names (for example, `BYTE_ORDER`) that on some +platforms come from system headers (for example, `` on FreeBSD). +Such checks have `!` after their names on the first line of their +implementation files (for example, `// BYTE_ORDER!`). + ## Adding new checks To add a check for a new configuration option `` simply create the -`.h` header file with the corresponding check and place it into -[`libbuild2/autoconf/checks/`][checks] (use existing checks for inspiration). +`.h` header file (preserving the case) with the corresponding check and +place it into [`libbuild2/autoconf/checks/`][checks] (use existing checks for +inspiration). The first line in this header file must be in the form: ``` -// +// [!] [: ...] ``` +If the name is followed by the `!` modifier, then it is *unprefixable* (see +the previous section for detail). The name can also be followed by `:` and a +list of base checks. Such checks are automatically inserted before the rest of +the lines in the resulting substitution. + Subsequent lines should be C-style comments or preprocessor directives that `#define` or `#undef` `` depending on whether the feature is available (though there can be idiosyncrasies; see `const.h`, for example). Note that there should be no double-quotes or backslashes except for line -continuations. For example: +continuations. For example, to add a check for option `HAVE_BAR`, we could +create the `HAVE_BAR.h` header file with the following content: ``` // HAVE_BAR @@ -148,10 +163,17 @@ continuations. For example: ``` Note also that the module implementation may need to replace `` with its -prefixed version if the `autoconf.prefix` functionality is in use (see above). -This is done by textually substituting every occurrence of `` that is -separated on both left and right hand sides (that is, both characters -immediately before and after `` are not `[A-Za-z0-9_]`). +prefixed version (unless it is unprefixable) if the `autoconf.prefix` +functionality is in use (see above). This is done by textually substituting +every occurrence of `` that is separated on both left and right hand +sides (that is, both characters immediately before and after `` are not +`[A-Za-z0-9_]`). + +Within a file duplicate checks are automatically suppressed. And if multiple +files are involved, then the user is expected to employ the `autoconf.prefix` +functionality to avoid clashes across files. However, this does not help +unprefixable names and, as a result, such checks should be implemented in +ways that deal with duplication (for example, include guards). [module-in]: https://build2.org/build2/doc/build2-build-system-manual.xhtml#module-in [proj-config]: https://build2.org/build2/doc/build2-build-system-manual.xhtml#proj-config diff --git a/libbuild2-autoconf-tests/basics/testscript b/libbuild2-autoconf-tests/basics/testscript index f19f3fd..63d1089 100644 --- a/libbuild2-autoconf-tests/basics/testscript +++ b/libbuild2-autoconf-tests/basics/testscript @@ -38,7 +38,10 @@ cat <=config.h.in; #undef ZERO #undef VALUE - # undef TRUE + #undef TRUE + #undef FALSE + + # undef MAYBE #undef CUSTOM_LINE #undef CUSTOM_BLOCK @@ -54,6 +57,7 @@ $* <>EOO #undef ZERO #define VALUE 123 - #define TRUE 1 + /* TRUE already defined. */ + /* FALSE already defined. */ + + #define MAYBE 1 #define CUSTOM 123 /* Make sure we do not redefine CUSTOM. */ @@ -101,15 +108,22 @@ cat <=config.h.cmake; #cmakedefine ZERO #cmakedefine VALUE - # cmakedefine TRUE + #cmakedefine TRUE + #cmakedefine FALSE + + # cmakedefine MAYBE #cmakedefine CUSTOM_LINE #cmakedefine CUSTOM_BLOCK - #cmakedefine CUSTOM_BLOCK @version@ + #cmakedefine CUSTOM_BLOCK1 @version@ - #cmakedefine TRUE true - #cmakedefine FALSE false - #cmakedefine VALUE @VALUE@ /* @version@ */ + #cmakedefine TRUE1 true + #cmakedefine FALSE1 false + #cmakedefine VALUE1 @VALUE1@ /* @version@ */ + + #cmakedefine TRUE1 true + #cmakedefine FALSE1 false + #cmakedefine VALUE1 @VALUE1@ /* @version@ */ #cmakedefine zzz_TEST_DUMMY1_H @zzz_TEST_DUMMY1_H@ #cmakedefine zzz_TEST_DUMMY2_H @@ -122,6 +136,7 @@ $* <>EOO #undef ZERO #define VALUE 123 - #define TRUE 1 + /* TRUE already defined. */ + /* FALSE already defined. */ + + #define MAYBE 1 #define CUSTOM 123 /* Make sure we do not redefine CUSTOM. */ #ifndef CUSTOM # define CUSTOM 123 #endif - /* Make sure we do not redefine CUSTOM. */ - #ifndef CUSTOM - # define CUSTOM 123 + /* Make sure we do not redefine CUSTOM1. */ + #ifndef CUSTOM1 + # define CUSTOM1 123 #endif - #define TRUE true - #undef FALSE - #define VALUE 123 /* 1.2.3 */ + #define TRUE1 true + #undef FALSE1 + #define VALUE1 123 /* 1.2.3 */ + + /* TRUE1 already defined. */ + /* FALSE1 already defined. */ + /* VALUE1 already defined. */ #define zzz_TEST_DUMMY1_H 1 #define zzz_TEST_DUMMY2_H 2 @@ -177,7 +211,10 @@ cat <=config.h.in; #mesondefine ZERO #mesondefine VALUE - # mesondefine TRUE + #mesondefine TRUE + #mesondefine FALSE + + # mesondefine MAYBE #mesondefine CUSTOM_LINE #mesondefine CUSTOM_BLOCK @@ -195,6 +232,7 @@ $* <>EOO #undef ZERO #define VALUE 123 - #define TRUE 1 + /* TRUE already defined. */ + /* FALSE already defined. */ + + #define MAYBE 1 #define CUSTOM 123 /* Make sure we do not redefine CUSTOM. */ @@ -236,6 +277,7 @@ ln -s ../../bootstrap.build ../../root.build build/; cat <=config.h.in; #undef PREFIX_zzz_TEST_DUMMY1_H #undef PREFIX_zzz_TEST_DUMMY2_H + #undef zzz_TEST_DUMMY3_H EOI $* <>EOO #define PREFIX_zzz_TEST_DUMMY1_H 1 #define PREFIX_zzz_TEST_DUMMY2_H 2 + #define zzz_TEST_DUMMY3_H 1 + EOO + +: base +: +mkdir build; +ln -s ../../bootstrap.build ../../root.build build/; +cat <=config.h.in; + #undef PREFIX_zzz_TEST_DUMMY4_H + EOI +$* <>EOO + #define PREFIX_zzz_TEST_DUMMY1_H 1 + + #define zzz_TEST_DUMMY3_H 1 + + #ifdef PREFIX_zzz_TEST_DUMMY1_H + # define PREFIX_zzz_TEST_DUMMY4_H zzz_TEST_DUMMY3_H + #endif /*PREFIX_zzz_TEST_DUMMY1_H*/ + EOO + +: base-duplicate +: +mkdir build; +ln -s ../../bootstrap.build ../../root.build build/; +cat <=config.h.in; + #undef PREFIX_zzz_TEST_DUMMY1_H + + #undef zzz_TEST_DUMMY3_H + + #undef PREFIX_zzz_TEST_DUMMY4_H + EOI +$* <>EOO + #define PREFIX_zzz_TEST_DUMMY1_H 1 + + #define zzz_TEST_DUMMY3_H 1 + + /* Base PREFIX_zzz_TEST_DUMMY1_H already defined. */ + + /* Base zzz_TEST_DUMMY3_H already defined. */ + + #ifdef PREFIX_zzz_TEST_DUMMY1_H + # define PREFIX_zzz_TEST_DUMMY4_H zzz_TEST_DUMMY3_H + #endif /*PREFIX_zzz_TEST_DUMMY1_H*/ + EOO + +: base-custom +: +mkdir build; +ln -s ../../bootstrap.build ../../root.build build/; +cat <=config.h.in; + #undef PREFIX_zzz_TEST_DUMMY4_H + EOI +$* <>EOO + #define PREFIX_zzz_TEST_DUMMY1_H 10 + + #define zzz_TEST_DUMMY3_H 30 + + #ifdef PREFIX_zzz_TEST_DUMMY1_H + # define PREFIX_zzz_TEST_DUMMY4_H zzz_TEST_DUMMY3_H + #endif /*PREFIX_zzz_TEST_DUMMY1_H*/ + EOO + +: base-recursive +: +mkdir build; +ln -s ../../bootstrap.build ../../root.build build/; +cat <=config.h.in; + #undef PREFIX_zzz_TEST_DUMMY5_H + EOI +$* <>EOO + #define zzz_TEST_DUMMY3_H 1 + + #define PREFIX_zzz_TEST_DUMMY1_H 1 + + /* Base zzz_TEST_DUMMY3_H already defined. */ + + #ifdef PREFIX_zzz_TEST_DUMMY1_H + # define PREFIX_zzz_TEST_DUMMY4_H zzz_TEST_DUMMY3_H + #endif /*PREFIX_zzz_TEST_DUMMY1_H*/ + + #ifdef zzz_TEST_DUMMY3_H + # define PREFIX_zzz_TEST_DUMMY5_H PREFIX_zzz_TEST_DUMMY4_H + #endif /*zzz_TEST_DUMMY3_H*/ EOO diff --git a/libbuild2-autoconf/libbuild2/autoconf/buildfile b/libbuild2-autoconf/libbuild2/autoconf/buildfile index 62f1ebc..2bb0a39 100644 --- a/libbuild2-autoconf/libbuild2/autoconf/buildfile +++ b/libbuild2-autoconf/libbuild2/autoconf/buildfile @@ -8,7 +8,7 @@ lib{build2-autoconf}: {hxx ixx txx cxx}{* -checks} {hxx cxx}{checks} \ $impl_libs $intf_libs # - File name must be the exact variable name plus .h (used for sorting). -# - First line in the file should be `// `. +# - First line in the file should be `// [] [:...]`. # - No double-quote or backslash escaping except for line continuations. # # NOTE: remember to update README.md if changing anything here. @@ -28,11 +28,11 @@ lib{build2-autoconf}: {hxx ixx txx cxx}{* -checks} {hxx cxx}{checks} \ h = $path($>[0]) s = $path($>[1]) - # We regularize the output with a dummy start entry plus add two end dummies - # that are used in tests. + # We regularize the output with a dummy start entry plus add five end + # dummies that are used in tests. # n = $size($<) - n += 3 + n += 6 cat <<"EOI" >$h namespace build2 @@ -42,6 +42,8 @@ lib{build2-autoconf}: {hxx ixx txx cxx}{* -checks} {hxx cxx}{checks} \ struct check { const char* name; + const char* modifier; // ! or empty + const char* base; // base names or empty const char* value; }; @@ -55,13 +57,13 @@ lib{build2-autoconf}: {hxx ixx txx cxx}{* -checks} {hxx cxx}{checks} \ const build2::autoconf::check build2::autoconf::checks[$n] = { { - \"\", + \"\", \"\", \"\", \"\" EOI cat $i | sed -n \ - -e 's|^// ([^ ]+) *$|},\n\n{\n"\1",\n|p' \ + -e 's|^// ([^ !:]+) *(!)? *(: *(( *[^ ]+)+))? *$|},\n\n{\n"\1", "\2", "\4",\n|p' \ -e 's|^(.*)\\$|"\1\\\\\\n"|p' \ -e 's|^(.*)$|"\1\\n"|p' \ - >>$s @@ -70,15 +72,37 @@ lib{build2-autoconf}: {hxx ixx txx cxx}{* -checks} {hxx cxx}{checks} \ }, { - "zzz_TEST_DUMMY1_H", + "zzz_TEST_DUMMY1_H", "", "", - "#define zzz_TEST_DUMMY1_H 1" + "#define zzz_TEST_DUMMY1_H 1\n" }, { - "zzz_TEST_DUMMY2_H", + "zzz_TEST_DUMMY2_H", "", "", - "#define zzz_TEST_DUMMY2_H 1" + "#define zzz_TEST_DUMMY2_H 1\n" + }, + + { + "zzz_TEST_DUMMY3_H", "!", "", + + "#define zzz_TEST_DUMMY3_H 1\n" + }, + + { + "zzz_TEST_DUMMY4_H", "", "zzz_TEST_DUMMY1_H zzz_TEST_DUMMY3_H", + + "#ifdef zzz_TEST_DUMMY1_H\n" + "# define zzz_TEST_DUMMY4_H zzz_TEST_DUMMY3_H\n" + "#endif /*zzz_TEST_DUMMY1_H*/\n" + }, + + { + "zzz_TEST_DUMMY5_H", "", "zzz_TEST_DUMMY3_H zzz_TEST_DUMMY4_H", + + "#ifdef zzz_TEST_DUMMY3_H\n" + "# define zzz_TEST_DUMMY5_H zzz_TEST_DUMMY4_H\n" + "#endif /*zzz_TEST_DUMMY3_H*/\n" } }; EOI diff --git a/libbuild2-autoconf/libbuild2/autoconf/rule.cxx b/libbuild2-autoconf/libbuild2/autoconf/rule.cxx index 2383004..a4efd15 100644 --- a/libbuild2-autoconf/libbuild2/autoconf/rule.cxx +++ b/libbuild2-autoconf/libbuild2/autoconf/rule.cxx @@ -1,6 +1,6 @@ #include -#include // strcmp() +#include // strcmp(), strchr() #include #include @@ -21,8 +21,9 @@ namespace build2 struct match_data { - autoconf::flavor flavor; - string prefix; + autoconf::flavor flavor; + string prefix; + map checks; // Checks already seen. }; rule:: @@ -78,7 +79,7 @@ namespace build2 // string p (cast_empty (t["autoconf.prefix"])); - t.data (match_data {f, move (p)}); + t.data (match_data {f, move (p), {}}); return r; } @@ -174,21 +175,34 @@ namespace build2 string& v (*ov); // As an extension, we allow replacing the entire line with a - // potentially multi-line block of preprocessor directives. To detect - // this, we look for a line that starts with `#` after whitespaces. + // potentially multi-line block of preprocessor directives or a + // single-line C comment (used to suppress duplicates). To detect the + // former, we look for a line that starts with `#` after whitespaces. + // To detect the latter, we look for a single line that starts with + // `/*`. // size_t p (0); for (;; ++p) { + bool c (p == 0); + skip_ws (v, p); if (v[p] == '#') break; + if (c) + c = (v[p] == '/' && v[p + 1] == '*'); + p = v.find ('\n', p); if (p == string::npos) + { + if (c) + p = 0; + break; + } } optional r; @@ -444,12 +458,24 @@ namespace build2 { assert (*flags == 1); + match_data& md (t.data ()); + // If this is a special substitution, then look in our catalog of // built-in checks. Specifically, the plan is as follows: // - // 1. Look in the catalog and fall through if not found. + // 1. Suppress if a duplicate (we do it regardless of whether it is + // from the catalog or custom). // - // 2. If found, then check for a custom value falling through if + // Which name should we store, prefixed or prefixless? If we store + // prefixed, then it won't be easy to match bases which are + // specified without a prefix: we will have to add the prefix + // unless the check is unprefixable, which we can only know by + // looking it up. So we store prefixless. Actually, it's convenient + // to store both. + // + // 2. Look in the catalog and fall through if not found. + // + // 3. If found, then check for a custom value falling through if // found. // // Here things get a bit tricky: while a stray HAVE_* buildfile @@ -460,15 +486,15 @@ namespace build2 // While this clashes with the in.null semantics, it's just as // easy to set the variable to the real default value as to null. // - // 3. Return the build-in value form the catalog. + // 4. Return the build-in value from the catalog. // - const char* pn (nullptr); // Prefix-less name. + const char* pn (nullptr); // Prefixless name. - const string& p (t.data ().prefix); + const string& p (md.prefix); if (!p.empty ()) { - // Note that if there is no prefix, then we don't look for a - // built-in check. + // Note that if there is no prefix, then we only look for an + // unprefixable built-in check. // if (n.size () > p.size () && n.compare (0, p.size (), p) == 0) pn = n.c_str () + p.size (); @@ -476,31 +502,69 @@ namespace build2 else pn = n.c_str (); - if (pn != nullptr) + const char* en (pn != nullptr ? pn : n.c_str ()); // Effective name. + + // Note: this line must be recognizable by substitute_special(). + // + if (md.checks.find (en) != md.checks.end ()) + return "/* " + n + " already defined. */"; + + md.checks.emplace (en, n); + + // If up is true, only look for an unprefixable check. + // + auto find = [] (const char* n, bool up = false) -> const check* { const check* e (checks + sizeof (checks) / sizeof (*checks)); const check* i (lower_bound (checks, e, - pn, - [] (const check& c, const char* pn) + n, + [] (const check& c, const char* n) { - return strcmp (c.name, pn) < 0; + return strcmp (c.name, n) < 0; })); - // Note: original name in lookup. + return (i != e && + strcmp (i->name, n) == 0 && + (!up || strchr (i->modifier, '!') != nullptr)) ? i : nullptr; + }; + + // Note: original name in variable lookup. + // + const check* c (find (en, pn == nullptr)); + + if (c != nullptr && !t[n]) + { + // The plan is as follows: keep adding base checks (suppressing + // duplicates) followed by the main check while prefixing all the + // already seen names (unless unprefixable). // - if (i != e && strcmp (i->name, pn) == 0 && !t[n]) + string r; + small_vector ns; + + auto append = [&r] (const char* v) { - string r (i->value); - - // Add "back" the prefix. + // Separate values with a blank line. // - if (!p.empty ()) + if (!r.empty ()) { - auto sep = [] (char c) { return !alnum (c) && c != '_'; }; + if (r.back () != '\n') + r += '\n'; - size_t m (n.size () - p.size ()); // Prefix-less name length. + r += '\n'; + } - for (size_t i (0); (i = r.find (pn, i)) != string::npos; i += m) + r += v; + }; + + auto prefix = [&p, &ns, &r, b = size_t (0)] () mutable + { + auto sep = [] (char c) { return !alnum (c) && c != '_'; }; + + for (const string& n: ns) + { + size_t m (n.size ()); // Prefix-less name length. + + for (size_t i (b); (i = r.find (n, i)) != string::npos; i += m) { if ((i == 0 || sep (r[i - 1])) && (i + m == r.size () || sep (r[i + m]))) @@ -511,8 +575,108 @@ namespace build2 } } - return r; + b = r.size (); + }; + + // Base checks. + // + // @@ TODO: detect cycles (currently we just prune, like an + // include guard). + // + // @@ TODO: add special mode (autoconf.test=true) that verifies + // on module load that all the bases are resolvable. + // Then add a test that triggers it (ideally we would + // want this at build-time, but that won't be easy in + // Buildscript). + // + auto base = [this, + &l, &t, a, &null, + &md, &p, &ns, + &find, &append, &prefix] (const string& n, + const char* bs, + const auto& base) -> void + { + auto df = make_diag_frame ( + [&n] (const diag_record& dr) + { + dr << info << "while resolving base options for " << n; + }); + + for (size_t b (0), e (0); next_word (bs, b, e); ) + { + string pn (bs, b, e - b); // Prefixless name. + + // First supress duplicates. + // + { + auto i (md.checks.find (pn)); + if (i != md.checks.end ()) + { + append (("/* Base " + + i->second + + " already defined. */").c_str ()); + + if (pn != i->second) + ns.push_back (move (pn)); + + continue; + } + } + + const check* c (find (pn.c_str ())); + + // While it may be overridden by the user, we should also have + // the entry in the built-in catalog (where we get the + // unprefixable flag). + // + if (c == nullptr) + fail (l) << "unknown base option " << pn; + + // Derive the prefixed name. + // + bool up (strchr (c->modifier, '!') != nullptr); + + string n ((up ? string () : p) + pn); + + md.checks.emplace (pn, n); + + if (t[n]) + append ( + this->in::rule::lookup (l, a, t, n, nullopt, null).c_str ()); + else + { + if (*c->base != '\0') + base (n, c->base, base); + + append (c->value); + } + + if (!p.empty ()) + { + if (!up) + ns.push_back (move (pn)); + + prefix (); + } + } + }; + + if (*c->base != '\0') + base (n, c->base, base); + + // Main check. + // + append (c->value); + + if (!p.empty ()) + { + if (pn != nullptr) // Not unprefixable. + ns.push_back (pn); + + prefix (); } + + return r; } }