lmi
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [lmi] C++ modernization


From: Greg Chicares
Subject: Re: [lmi] C++ modernization
Date: Tue, 10 Jan 2017 15:41:30 +0000
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.5.1

On 2017-01-10 11:54, Greg Chicares wrote:
> On 2017-01-10 04:46, Greg Chicares wrote:
>> On 2017-01-09 16:49, Vadim Zeitlin wrote:
>> [...]
>>> Personally I'd prefer if you could have a look at the pending ones
>>> (https://github.com/vadz/lmi/pull/46, 47 and 48), especially because the
>>> likelihood of conflicts when doing such global changes is pretty high.
> 
> [snip pull/46 discussion]
> 
> Moving on to pull/47...

And now pull/48.

I'm really glad you limited this changeset to files that use <regex>,
because that makes the most compelling case for raw strings. I'm not
yet sure we should change isolated lines like this:
  configurable_settings_test.cpp:        << "<?xml version=\"1.0\"?>\n"
but I do think we should change files that write html.

Here's the only hunk that didn't apply (because I had split it into
two tests in the meantime):

--------8<--------8<--------8<--------8<--------8<--------8<--------
$cat test_coding_rules.cpp.rej
--- test_coding_rules.cpp
+++ test_coding_rules.cpp
@@ -319,7 +319,7 @@ void assay_whitespace(file const& f)
         ||  contains(f.data(), '\v')
         )
         {
-        throw std::runtime_error("File contains '\\r' or '\\v'.");
+        throw std::runtime_error(R"(File contains '\r' or '\v'.)");
         }
 
     if
-------->8-------->8-------->8-------->8-------->8-------->8--------

That was so simple for me to adjust that it would be a waste of time
to ask you to review it...NOT.

  throw std::runtime_error(R"File contains '\r'.");

Fortunately, gcc's error messages are clear as always...NOT:

/opt/lmi/src/lmi/test_coding_rules.cpp:321:41: error: invalid character ' ' in 
raw string delimiter
         throw std::runtime_error(R"File contains '\r'.");
                                  ^

(that makes me wish my old EDG compiler supported C++11), but it
wasn't that hard to figure out the correct change (and to understand
why "R" has to behave differently than "L").

And if "R" and "L" behave differently, then what does a "Y" prefix do?
+    std::string const Y(R"((( +| *, *)(address@hidden 
*[0-9]+|[a-z]+|[\[\(][^;]+[\]\)])))");
Then I realized: it designates obfuscated perl.

But, once we get past those comic asides, we come to a real problem:

/opt/lmi/src/lmi[0]$make $coefficiency check_concinnity              
make[1]: Entering directory '/opt/lmi/src/lmi'
make[2]: 'test_coding_rules.exe' is up to date.
make[1]: Leaving directory '/opt/lmi/src/lmi'
  Problems detected by xmllint:
  Miscellaneous problems:
File 'test_coding_rules.cpp' must not include 'config.hpp'.

Happily we observe that the regex flavor in this source is similar
enough to vim's that we can just type this:

/# *include *[<\"]config.hpp[>\"]

and find six lines that now match.

I could re-cook the literals, but that's nasty. I thought of making
the ten-character string {config.hpp} a macro, because it looks like
it's not quoted...huh?...no, it's just that I haven't upgraded to
vim++11 so relying on highlighting misled me. Maybe I could rewrite
it as /[#]config.hpp/ ...

...which led me to try to write a regex to match the set of all
regexes that do not match themselves, so that we could automate the
detection of problems like this...and to consider whether it would
match itself...

...and then I awoke and did what I hope is the sensible thing, but
maybe you should review it nevertheless.

I won't even mention that I got this recently-mentioned error again
when I ran the full unit-test suite:

Running product_file_test:

** uncaught exception: std::runtime_error: Unable to parse xml file 
'Z:/opt/lmi/src/build/lmi/Linux/gcc/ship/sample.policy': Document is empty
[file /opt/lmi/src/lmi/xml_lmi.cpp, line 69]




reply via email to

[Prev in Thread] Current Thread [Next in Thread]