generic classes for safe %macro substitution

Oswald Buddenhagen ossi at kde.org
Thu Jan 2 05:01:24 GMT 2003


On Wed, Jan 01, 2003 at 08:28:08PM +0100, Harri Porten wrote:
> > the class names are somewhat scary.
> 
> What I find slightly "scary" is the _number_ of classes. Can't they
> maybe be merged into one class with different function names or flags
> denoting the different behaviour ?
> 
> my initial feeling was that a set of static functions would do the job
> equally well (especially considering the low numbers of data members
> and the lack of any state other than the map passed to the ctor).
> 
one might think so ... but the whole thing is supposed to be extensible.
it should be possible to derive a class that provides own expand*Macro
functions (i want to use this to improve kconfig's shell substitutions).
therefore we have at least two data sets: the callback functions (also
called the vmt :) and the parameters they need. it would be possible to
merge the two map expander classes into one, but that does not remove
the need for a data container. therefore it does not serve us much to do
it all static (one would have to use classic callbacks then). the
alternative would be replicating the main loop and the whole shell
parser into every possible substitution function, but that's beyond
reason.

> A QString parameter doesn't really warrant an in-place modification.
>
the (to be written) documentation does.

> Returning the processed value often eases the usage
>
certainly.

> and doesn't cost much more.
>
have you looked at the code generated for returning temporaries? things
would look different if gcc realized that a qstring is actually only a
pointer, but it does not ...

> Even your kmxtest.cpp program contains extra copy statements because
> of this :)
> 
it would need some either way ...

i decided for the in-place modification mainly because it's consistent
with the pos parameter and because it fits the prospective application 
in kconfig.

>     enum Quoting { noquote, singlequote, doublequote, dollarquote, 
> 		parent, subst, group, math };
> 
> As the Save and State structs are only used within a private function you
> might as well move their definitions into the .cpp file (not that elegant
> I know, but save the compiler from parsing those each time the .h is
> included somewhere).
> 
originally i had them local to the function using them, but then gcc
complained about qvaluestack instantiation with a local type. i didn't
try declaring them in the global scope, though.

>     void setMacroMap( const QMap<QString,QString> &map );
>     QMap<QString,QString> &macroMap() const;
> 
> You can return a value here without worrying about the costs (shallow
> copy).
>
object returns by value are always relatively expensive. expecially as
this function is also used internally.

> Returning a reference gives the user direct access to private data
> and thus breaks encapsulation.
> 
that's a point, though ...

>     void expandMacros( QString &str ) { KMacroExpander::expandMacros( str
> ); }
>     bool expandMacrosShellQuote( QString &str ) { return
> KMacroExpander::expandMacrosShellQuote( str ); }
>     bool expandMacrosShellQuote( QString &str, uint &pos ) { return
> KMacroExpander::expandMacrosShellQuote( str, pos ); }
> 
> These functions can all be "const", can't they ?
> 
if i knew exactly what the keyword means, i could decide ... :}

>     bool isIdentifier(uint c) { return c == '_' || (c >= 'A' && c <= 'Z')
> || (c >= 'a' && c <= 'z') || (c >= '0' && c <= '9'); }
> };
> 
> "const" :)
> 
whatever. the function is inline, so i doubt we can tell the compiler
more than it can find out by itself. otoh, g++ already surprised me with
its stupidity.

greetings

-- 
Hi! I'm a .signature virus! Copy me into your ~/.signature, please!
--
Chaos, panic, and disorder - my work here is done.




More information about the kde-core-devel mailing list