KDE/kdevelop/languages/cpp

[Matt Rogers]

On Sunday 08 June 2008 15:30:31 Andreas Pakulat wrote:
> SVN commit 818572 by apaku:
>
> Fix the kwarning madness here.
> Note: kWarning() is for user-visible strings, i.e. "file foo for
> parsing not found" and similar stuff. Not for any debug output (like
> "multiple class contexts found").
> So any output that a user doesn't understand (no kdevelop user has an idea
> about ducontext) should use kDebug() not kWarning()
> CCMAIL:kdevelop-devel at kdevelop.org
>
>
>  M  +1 -1      codecompletioncontext.cpp
>  M  +3 -3      cppduchain/contextbuilder.cpp
>  M  +5 -5      cppduchain/cpptypes.cpp
>  M  +2 -2      cppduchain/templatedeclaration.cpp
>  M  +1 -1      cppduchain/typerepository.cpp
>  M  +1 -1      cpplanguagesupport.cpp
>  M  +1 -1      debugger/framestackwidget.cpp
>  M  +1 -1      parser/rpp/pp-environment.cpp
>  M  +1 -1      preprocessjob.cpp
>
>
> --- trunk/KDE/kdevelop/languages/cpp/codecompletioncontext.cpp
> #818571:818572 @@ -563,7 +563,7 @@
>    if( m_duContext ) {
>    m_text = preprocess( m_text, 
> dynamic_cast<Cpp::EnvironmentFile*>(m_duContext->topContext()->parsingEnvir
>onmentFile().data()), line ); }else{
> -    kWarning() << "error: no ducontext";
> +    kDebug() << "error: no ducontext";

should be a warning still. 

>    }
>  }
>
> --- trunk/KDE/kdevelop/languages/cpp/cppduchain/contextbuilder.cpp
> #818571:818572 @@ -264,7 +264,7 @@
>
>      if( topLevelContext && !topLevelContext->smartRange() &&
> m_editor->smart() ) { lock.unlock();
> -      kWarning() << "Smartening Context!";
> +      kDebug() << "Smartening Context!";

this is fine.

>        smartenContext(topLevelContext);
>        lock.lock();
>        topLevelContext = updateContext.data(); //In case the context was
> deleted, updateContext as a DUChainPointer will have noticed it. @@ -351,7
> +351,7 @@
>
>    if (!m_importedParentContexts.isEmpty()) {
>      DUChainReadLocker lock(DUChain::lock());
> -    kWarning(9007) << file->url().str() << "Previous parameter declaration
> context didn't get used??" ; +    kDebug(9007) << file->url().str() <<
> "Previous parameter declaration context didn't get used??" ; DumpChain
> dump;
>      dump.dump(topLevelContext);
>      m_importedParentContexts.clear();
> @@ -501,7 +501,7 @@
>        if (classContexts.count() != 0)
>          m_importedParentContexts.append(classContexts.first());
>        if (classContexts.count() > 1) {
> -        kWarning(9007) << "Muliple class contexts for" <<
> className.toString() << "- shouldn't happen!" ; 
> +        kDebug(9007) <<
> "Muliple class contexts for" << className.toString() << "- shouldn't
> happen!" ; foreach (DUContext* classContext, classContexts) {

needs to be a warning still. useful in a bug report if it happens.

>            kDebug(9007) << "Context" << classContext->scopeIdentifier(true)
> << "range" << classContext->range().textRange() << "in" <<
> classContext->url().str(); }
> --- trunk/KDE/kdevelop/languages/cpp/cppduchain/cpptypes.cpp #818571:818572
> @@ -306,28 +306,28 @@
>  template<>
>  void CppConstantIntegralType::setValueInternal<qint64>(qint64 value) {
>    if((typeModifiers() & ModifierUnsigned))
> -    kWarning() << "setValue(signed) called on unsigned type";
> +    kDebug() << "setValue(signed) called on unsigned type";

fine. 
>    m_value = value;
>  }
>
>  template<>
>  void CppConstantIntegralType::setValueInternal<quint64>(quint64 value) {
>    if(!(typeModifiers() & ModifierUnsigned))
> -    kWarning() << "setValue(unsigned) called on not unsigned type";
> +    kDebug() << "setValue(unsigned) called on not unsigned type";

fine

>    m_value = (qint64)value;
>  }
>
>  template<>
>  void CppConstantIntegralType::setValueInternal<float>(float value) {
>    if(integralType() != TypeFloat)
> -    kWarning() << "setValue(float) called on non-float type";
> +    kDebug() << "setValue(float) called on non-float type";

fine

>    memcpy(&m_value, &value, sizeof(float));
>  }
>
>  template<>
>  void CppConstantIntegralType::setValueInternal<double>(double value) {
>    if(integralType() != TypeDouble)
> -    kWarning() << "setValue(double) called on non-double type";
> +    kDebug() << "setValue(double) called on non-double type";

fine

>    memcpy(&m_value, &value, sizeof(double));
>  }
>
> @@ -522,7 +522,7 @@
>  void CppClassType::addElement(AbstractType::Ptr element)
>  {
>    if (isClosed())
> -    kWarning(9007) << "Tried to add type" << element->toString() << "to
> closed class!" ; 
> +    kDebug(9007) << "Tried to add type" <<
> element->toString() << "to closed class!" ; else

not sure.

>      StructureType::addElement(element);
>  }
> --- trunk/KDE/kdevelop/languages/cpp/cppduchain/templatedeclaration.cpp
> #818571:818572 @@ -449,7 +449,7 @@
>            if( instantiatedDeclaration )
>              tempDecl->setTemplateParameterContext( ctx );
>            else
> -            kWarning(9007) << "instantiated declaration is not a template
> declaration"; +            kDebug(9007) << "instantiated declaration is not
> a template declaration"; }
>        }
>        else
> @@ -640,7 +640,7 @@
>            return 0;
>        }
>      }else{
> -      kWarning(9007) << "Problem in template forward-declaration";
> +      kDebug(9007) << "Problem in template forward-declaration";

This should be reported by the problem reporter, IMHO.

>        return 0;
>      }
>    }else{
> --- trunk/KDE/kdevelop/languages/cpp/cppduchain/typerepository.cpp
> #818571:818572 @@ -173,7 +173,7 @@
>  AbstractType::Ptr TypeRepository::registerType(AbstractType::Ptr input)
>  {
>    if (!input) {
> -    kWarning(9007) << "Asked to register a null type." ;
> +    kDebug(9007) << "Asked to register a null type." ;

needs to be a warning

>      return input;
>    }
>
> --- trunk/KDE/kdevelop/languages/cpp/cpplanguagesupport.cpp #818571:818572
> @@ -380,7 +380,7 @@
>      if(def->internalContext()) {
>        targetRange.end() =
> def->internalContext()->range().end.textCursor(); }else{
> -      kWarning(9007) << "Declaration does not have internal context";
> +      kDebug(9007) << "Declaration does not have internal context";

needs to be a warning
>      }
>      lock.unlock();
>
> --- trunk/KDE/kdevelop/languages/cpp/debugger/framestackwidget.cpp
> #818571:818572 @@ -102,7 +102,7 @@
>          return;
>
>      if (selected.count() > 1) {
> -        kWarning() << "Selection not single as requested";
> +        kDebug() << "Selection not single as requested";

needs to be a warning

>          return;
>      }
>
> --- trunk/KDE/kdevelop/languages/cpp/parser/rpp/pp-environment.cpp
> #818571:818572 @@ -82,7 +82,7 @@
>  {
>    if (depth++ > 100) {
>      // TODO detect recursion?
> -    kWarning(9007) << "Likely cyclic include, aborting macro replay at
> depth 100" ; +    kDebug(9007) << "Likely cyclic include, aborting macro
> replay at depth 100" ; return;
>    }

should be a warning.

>
> --- trunk/KDE/kdevelop/languages/cpp/preprocessjob.cpp #818571:818572
> @@ -488,7 +488,7 @@
>
>      } else {
>          // What... the parent job got deleted??
> -        kWarning(9007) << "Parent job disappeared!!" ;
> +        kDebug(9007) << "Parent job disappeared!!" ;

needs to be a warning
>          m_success = false;
>          setFinished(true);
>          return true;
>

I think we went a little bit overboard here. Messages for things that should 
never happen or are going to be useful for quickly pointing us in the right 
direction to solve bugs can be warnings. If they're going to be user visible 
though, we might want to clean them up a little bit.
--
Matt