Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

type qualifiers ignored on function return type [-Werror=ignored-qualifiers] #746

Closed
drizt opened this issue Aug 20, 2018 · 6 comments
Closed

Comments

@drizt
Copy link
Contributor

drizt commented Aug 20, 2018

My application produce such errors with tidy

C:/psi-root/include/tidy.h:2095:88: error: type qualifiers ignored on function return type [-Werror=ignored-qualifiers]
 TIDY_EXPORT const ctmbstr TIDY_CALL TidyLangWindowsName( const tidyLocaleMapItem *item );
C:/psi-root/include/tidy.h:2101:86: error: type qualifiers ignored on function return type [-Werror=ignored-qualifiers]
 TIDY_EXPORT const ctmbstr TIDY_CALL TidyLangPosixName( const tidyLocaleMapItem *item );

In this line TIDY_EXPORT const ctmbstr TIDY_CALL TidyLangWindowsName( const tidyLocaleMapItem *item ); ctmbstr has allready defined as const so no need to add extra const to function declaration. The same for another line.

drizt added a commit to drizt/tidy-html5 that referenced this issue Aug 20, 2018
@geoffmcl
Copy link
Contributor

@drizt while I do not get this warning, agree it does appear an accidental duplication of the const qualifier... ctmbstr is already type-defined with a const...

Was just about to merge #767, but noted this should be repeated in libtidy.c, and the matching by name internal TY_() calls in language.c|h...

This gives a full fix as follows - have I missed any other cases?

diff --git a/include/tidy.h b/include/tidy.h
index c701890..7e30216 100644
--- a/include/tidy.h
+++ b/include/tidy.h
@@ -2092,13 +2092,13 @@ TIDY_EXPORT const tidyLocaleMapItem* TIDY_CALL getNextWindowsLanguage( TidyItera
  ** @param item An instance of tidyLocalMapItem to query.
  ** @result Returns a string with the Windows name of the mapping.
  */
-TIDY_EXPORT const ctmbstr TIDY_CALL TidyLangWindowsName( const tidyLocaleMapItem *item );
+TIDY_EXPORT ctmbstr TIDY_CALL TidyLangWindowsName( const tidyLocaleMapItem *item );
 
 /** Given a `tidyLocalMapItem`, return the POSIX name.
  ** @param item An instance of tidyLocalMapItem to query.
  ** @result Returns a string with the POSIX name of the mapping.
  */
-TIDY_EXPORT const ctmbstr TIDY_CALL TidyLangPosixName( const tidyLocaleMapItem *item );
+TIDY_EXPORT ctmbstr TIDY_CALL TidyLangPosixName( const tidyLocaleMapItem *item );
 
 /** @}
  ** @name Getting Localized Strings
diff --git a/src/language.c b/src/language.c
index fd89730..3907d9e 100644
--- a/src/language.c
+++ b/src/language.c
@@ -593,7 +593,7 @@ const tidyLocaleMapItemImpl *TY_(getNextWindowsLanguage)( TidyIterator *iter )
 /**
  *  Given a `tidyLocalMapItemImpl, return the Windows name.
  */
-const ctmbstr TY_(TidyLangWindowsName)( const tidyLocaleMapItemImpl *item )
+ctmbstr TY_(TidyLangWindowsName)( const tidyLocaleMapItemImpl *item )
 {
     return item->winName;
 }
@@ -602,7 +602,7 @@ const ctmbstr TY_(TidyLangWindowsName)( const tidyLocaleMapItemImpl *item )
 /**
  *  Given a `tidyLocalMapItemImpl, return the POSIX name.
  */
-const ctmbstr TY_(TidyLangPosixName)( const tidyLocaleMapItemImpl *item )
+ctmbstr TY_(TidyLangPosixName)( const tidyLocaleMapItemImpl *item )
 {
     return item->POSIXName;
 }
diff --git a/src/language.h b/src/language.h
index c8542b5..7ff4d18 100644
--- a/src/language.h
+++ b/src/language.h
@@ -191,12 +191,12 @@ const tidyLocaleMapItemImpl *TY_(getNextWindowsLanguage)( TidyIterator* iter );
 /**
  *  Given a `tidyLocalMapItemImpl, return the Windows name.
  */
-const ctmbstr TY_(TidyLangWindowsName)( const tidyLocaleMapItemImpl *item );
+ctmbstr TY_(TidyLangWindowsName)( const tidyLocaleMapItemImpl *item );
 
 /**
  *  Given a `tidyLocalMapItemImpl, return the POSIX name.
  */
-const ctmbstr TY_(TidyLangPosixName)( const tidyLocaleMapItemImpl *item );
+ctmbstr TY_(TidyLangPosixName)( const tidyLocaleMapItemImpl *item );
 
 /**
  *  Initializes the TidyIterator to point to the first item
diff --git a/src/tidylib.c b/src/tidylib.c
index 31754ab..823ec3b 100644
--- a/src/tidylib.c
+++ b/src/tidylib.c
@@ -2644,13 +2644,13 @@ const tidyLocaleMapItem* TIDY_CALL getNextWindowsLanguage( TidyIterator* iter )
 }
 
 
-const ctmbstr TIDY_CALL TidyLangWindowsName( const tidyLocaleMapItem *item )
+ctmbstr TIDY_CALL TidyLangWindowsName( const tidyLocaleMapItem *item )
 {
     return TY_(TidyLangWindowsName)( (tidyLocaleMapItemImpl*)(item) );
 }
 
 
-const ctmbstr TIDY_CALL TidyLangPosixName( const tidyLocaleMapItem *item )
+ctmbstr TIDY_CALL TidyLangPosixName( const tidyLocaleMapItem *item )
 {
     return TY_(TidyLangPosixName)( (tidyLocaleMapItemImpl*)(item) );
 }

Hopefully someone will patch and test, and report - it compiles fine in WIN32.X64 - but need others to confirm...

Then maybe the additional fixes could be added to the PR, or a separate simultaneous PR... thanks...

@drizt
Copy link
Contributor Author

drizt commented Nov 20, 2018

I use Clang to compile and turn all warnings to errors. Clang has more strict syntax checking than GCC.

@drizt
Copy link
Contributor Author

drizt commented Nov 20, 2018

Try set( WARNING_FLAGS "-Wall -Wextra") with Clang or GCC. I don't know how do the same with MSVC.

@geoffmcl
Copy link
Contributor

@drizt WOW. adding -Wextra, like $ cmake -DCMAKE_C_FLAGS=-Wextra ... increased the WARNING lines to nearly 2,000! YUK! of course many are repeated...

And there are about 160 lines in my log, with the warning: type qualifiers ignored ..., that you are reporting... and includes the above 8 cases in the patch...

The question becomes should we be worried about most of these?

One very common, is like warning: missing initializer for the field 'pdflt'... - yes, we do have some structures, where not all initializes have been given, and have always understood the value used would be null, but I do not think that is important, since we do not depend on its init value anywhere in the code... but maybe???

On the other hand, in the specific case you mention, it does seem wrong to repeat the const like that... I think that is just an honest typo type error... and should be fixed... in all 8 cases...

Look forward to further feedback on all the other warnings that the -Wextra gcc flag exposes... maybe as separate issues...

Now, the above patch could be included in your PR #747, if you get the chance, and really appreciate the help, especially with Clang testing...

or I could merge, and add the others later...

Now that I have seen more on this issue, will try to get to at least this const fix soonest... thanks...

@drizt
Copy link
Contributor Author

drizt commented Nov 27, 2018

-Wextra is important for .h files becouse other soft can use such flag. And other soft will get errors as me. In .c files will good to fix this but not real sense to do this. In most cases these errors not point to real problem.

Ok. I improve my PR when I get enough free time.

geoffmcl added a commit that referenced this issue Jan 14, 2019
Fix extra const modifier
geoffmcl added a commit that referenced this issue Jan 14, 2019
@geoffmcl
Copy link
Contributor

@drizt while there still remains some warnings with -DCMAKE_C_FLAGS=-Wextra, this problem has certainly been fixed in the public tidy.h header...

Had to also make some fixes in the matching *.c declarations, since MSVC140 flags this as an error...

Thanks for the issue and PR...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants